V3 Upgrade : ESM, TypeScript 4.9 & Node.js support

[gfortaine]

• feat(upgrade): migrate to ESM
• feat(upgrade): add exports
• feat(upgrade): upgrade to TypeScript 4.9
• feat(upgrade): add node.js support
• feat(upgrade): adjust parse.getMessages signature

[gfortaine]

cc @vishwam

[gfortaine]

Here is the package 🎉 : https://www.npmjs.com/package/@fortaine/fetch-event-source

[lexich]

Hey, @gfortaine, you've done a perfect job. Is it strictly required to use "node": ">=18.12" I want to use your package while official is in progress, but I use node16. Can you downgrade this requirement?

[gfortaine]

@lexich Done (Node v16.15.0 (LTS) w/ experimental support to the fetch API) ✅ : https://www.npmjs.com/package/@fortaine/fetch-event-source/v/3.0.5?activeTab=explore

[jordn]

Thank you very much @gfortaine! Just what I was hoping for.

[gfortaine]

@firedog1024 Wouldn't Microsoft mind to prioritize this PR, please ?

[waylaidwanderer]

@gfortaine On Node 16.15.1, with a node-fetch polyfill, it gives the following error:

TypeError: stream.getReader is not a function                                                                                                                                                                                                                                         
    at getBytes (file:///.../node_modules/@fortaine/fetch-event-source/lib/esm/parse.js:2:27)
    at create (file:///.../node_modules/@fortaine/fetch-event-source/lib/esm/fetch.js:54:23)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Edit: replacing getBytes with this seems to fix it:

export async function getBytes(body, onChunk) {
    for await (const chunk of body) {
        onChunk(chunk);
    }
}

But a check should be made to see if getReader is a function first and use the appropriate version of the code.

[transitive-bullshit]

Here's some additional Node.js-specific cases to be aware of: https://github.com/transitive-bullshit/chatgpt-api/blob/main/src/fetch-sse.ts#L27-L48

@gfortaine you may want to just publish your fork's source as your package's source. Microsoft projects aren't always the best at incorporating community PRs.

[julien-c]

would be awesome to get this merged, IMO

[hperrin]

@waylaidwanderer Your fix works a treat!

I added that fix to @gfortaine's fixes and published them here: https://www.npmjs.com/package/fetch-event-source-hperrin

[BuddhaBing]

@vishwam can we get this merged please?

[BuddhaBing]

@gfortaine can you add this fix to your PR please (and ideally your package also, since I'm currently using that until this PR is merged)

[eugeneYWang]

If this can be be fixed and released, many people, including me, would be helped !

[eugeneYWang]

@gfortaine can you add this fix to your PR please (and ideally your package also, since I'm currently using that until this PR is merged)

Fontaine's lib would work well under NodeJS 18.17.1. Even without the fix that you referred to. Guess Native Fetch implementation has been improved, and connection header seems to be not forbidden as well

[BuddhaBing]

@vishwam @firedog1024 has this project been abandoned? What's the issue with getting this merged? Clearly a lot of people are eager for it, given the comments here

[dargmuesli]

You might want to check out extended-eventsource as potential new replacement. It's codebase looks proper.

[andresgutgon]

What's the status of this PR?

[dargmuesli]

Please don't post redundant comments, it pings everyone interested in actually resolving this issue without adding much value. Let's spend that time on PRs for maintained projects 🙏

[andresgutgon]

Ok but what would be the right way to know if this is moving forward? Sorry for the noise

[dargmuesli]

You would see it happening here.