-
Notifications
You must be signed in to change notification settings - Fork 179
V3 Upgrade : ESM, TypeScript 4.9 & Node.js support #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cc @vishwam
a69c594
to
60183b2
Compare
60183b2
to
956ed99
Compare
344365b
to
b8f0a4e
Compare
Here is the package 🎉 : https://www.npmjs.com/package/@fortaine/fetch-event-source
lexich
commented
Jan 11, 2023
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?
b8f0a4e
to
26111d6
Compare
@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
commented
Jan 12, 2023
Thank you very much @gfortaine! Just what I was hoping for.
26111d6
to
43a34d8
Compare
43a34d8
to
3af438f
Compare
@firedog1024 Wouldn't Microsoft mind to prioritize this PR, please ?
@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
commented
Feb 8, 2023
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
commented
Apr 5, 2023
would be awesome to get this merged, IMO
hperrin
commented
Jun 13, 2023
@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
commented
Jul 17, 2023
@vishwam can we get this merged please?
BuddhaBing
commented
Jul 17, 2023
@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
commented
Aug 16, 2023
If this can be be fixed and released, many people, including me, would be helped !
eugeneYWang
commented
Aug 26, 2023
@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
commented
Sep 6, 2023
@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
commented
Feb 21, 2024
You might want to check out extended-eventsource as potential new replacement. It's codebase looks proper.
andresgutgon
commented
Oct 16, 2024
What's the status of this PR?
dargmuesli
commented
Oct 16, 2024
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
commented
Oct 16, 2024
Ok but what would be the right way to know if this is moving forward? Sorry for the noise
dargmuesli
commented
Oct 16, 2024
You would see it happening here.
Uh oh!
There was an error while loading. Please reload this page.