Skip to content
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

Rewrite module to use fetch, WebStreams, TypeScript and ESM #330

Merged
merged 11 commits into from
Dec 7, 2024
Merged

Conversation

rexxars
Copy link
Member

@rexxars rexxars commented Nov 15, 2024

Full background: #329

Deno tests are failing because it doesn't handle redirects correctly. I've filed a security issue with them - I'd rather not implementing fixes for incorrect fetch implementations in this module. My PR to Deno landed and was released in 2.1.2 - tests are now passing

BREAKING CHANGE: The module now uses a named export instead of a default export.

BREAKING CHANGE: UMD bundle dropped. Use a bundler.

BREAKING CHANGE: `headers` in init dict dropped, pass a custom `fetch` function instead.

BREAKING CHANGE: HTTP/HTTPS proxy support dropped. Pass a custom `fetch` function instead.

BREAKING CHANGE: `https.*` options dropped. Pass a custom `fetch` function that provides an agent/dispatcher instead.

BREAKING CHANGE: New default reconnect delay: 3 seconds instead of 1 second.

BREAKING CHANGE: Reconnecting after a redirect will now always use the original URL, even if the status code was HTTP 307.
@aslakhellesoy
Copy link
Contributor

Oh wow.

I'm going to need some time to digest all this and give it a proper review. I agree 100% with you that this module needs modernisation.

Thanks a lot for doing all this. If I don't reply within a week, don't let that hold this up. I know from experience that your work is top notch @rexxars.

@rexxars
Copy link
Member Author

rexxars commented Nov 15, 2024

I'm going to need some time to digest all this and give it a proper review. I agree 100% with you that this module needs modernisation.

No rush! I realize there are a lot of changes, tooling etc that makes this harder to review. This is also a big change and dropping compatibility with older runtimes isn't something I take lightly. This will be a major version bump, and users are free to keep using the old version.

I am trying out the prerelease (eventsource@next) in a couple of apps to see that things are working as expected, so until I have more data I am in no rush to merge.

@rexxars rexxars merged commit f9b7c90 into master Dec 7, 2024
7 checks passed
@rexxars rexxars deleted the v3 branch December 7, 2024 17:08
rexxars added a commit that referenced this pull request Dec 7, 2024
BREAKING CHANGE: The module now uses a named export instead of a default export.

BREAKING CHANGE: UMD bundle dropped. Use a bundler.

BREAKING CHANGE: `headers` in init dict dropped, pass a custom `fetch` function instead.

BREAKING CHANGE: HTTP/HTTPS proxy support dropped. Pass a custom `fetch` function instead.

BREAKING CHANGE: `https.*` options dropped. Pass a custom `fetch` function that provides an agent/dispatcher instead.

BREAKING CHANGE: New default reconnect delay: 3 seconds instead of 1 second.

BREAKING CHANGE: Reconnecting after a redirect will now always use the original URL, even if the status code was HTTP 307.
rexxars added a commit that referenced this pull request Dec 7, 2024
BREAKING CHANGE: Drop support for Node.js versions below v18

BREAKING CHANGE: The module now uses a named export instead of a default export.

BREAKING CHANGE: UMD bundle dropped. Use a bundler.

BREAKING CHANGE: `headers` in init dict dropped, pass a custom `fetch` function instead.

BREAKING CHANGE: HTTP/HTTPS proxy support dropped. Pass a custom `fetch` function instead.

BREAKING CHANGE: `https.*` options dropped. Pass a custom `fetch` function that provides an agent/dispatcher instead.

BREAKING CHANGE: New default reconnect delay: 3 seconds instead of 1 second.

BREAKING CHANGE: Reconnecting after a redirect will now always use the original URL, even if the status code was HTTP 307.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants