Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares a breaking v5 release by adding a parser buffer limit, moving the package/tooling to modern ESM/Node 22.19+ assumptions, and updating runtime support documentation.
Changes:
- Adds configurable
maxBufferSizehandling for parser buffer overflow and tests its failure behavior. - Drops CommonJS packaging paths and updates TypeScript/module/import configuration for
.tssource imports. - Updates tooling, CI/runtime targets, cleanup/formatting setup, and migration/security documentation.
Reviewed changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/EventSource.ts |
Wires parser maxBufferSize and fails the connection on max-buffer parse errors. |
src/types.ts |
Adds maxBufferSize to EventSourceInit. |
src/index.ts |
Updates source exports to .ts extensions. |
test/tests.ts |
Adds coverage for custom maxBufferSize overflow behavior. |
test/type-compatible.ts |
Adds type compatibility check for maxBufferSize. |
test/helpers.ts |
Updates source import extension. |
test/server.ts |
Updates fixture import extension. |
test/node/client.node.test.ts |
Updates test imports to .ts extensions. |
test/deno/client.deno.test.ts |
Updates test imports to .ts extensions. |
test/bun/client.bun.test.ts |
Updates test imports to .ts extensions. |
test/browser/client.browser.test.ts |
Updates browser test imports to .ts extensions. |
test/browser/browser-test.ts |
Updates browser bundle imports to .ts extensions. |
test/waffletest/index.ts |
Updates re-exports to .ts extensions. |
test/waffletest/runner.ts |
Updates internal test runner imports. |
test/waffletest/reporters/defaultReporter.ts |
Updates reporter imports. |
test/waffletest/reporters/helpers.ts |
Updates reporter type import. |
test/waffletest/reporters/nodeReporter.ts |
Updates reporter imports. |
tsconfig.settings.json |
Raises target/lib and enables TS extension rewriting/module syntax options. |
tsconfig.dist.json |
Enables noCheck for distribution type build. |
package.json |
Updates package exports, scripts, dependencies, engines, and browser targets. |
package.config.ts |
Removes bundle visualizer config and keeps dist tsconfig. |
scripts/clean.ts |
Adds Node-based cleanup script. |
.oxfmtrc.json |
Adds formatter configuration. |
.github/workflows/test.yml |
Updates Node/action versions and test matrix. |
.github/workflows/release.yml |
Updates release workflow setup-node usage. |
README.md |
Updates supported engines and documents parser buffer limit. |
MIGRATION.md |
Adds v4-to-v5 migration notes. |
SECURITY.md |
Updates supported version table. |
CONTRIBUTING.md |
Removes an extra blank list line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BREAKING CHANGE: The client will now fail the connection (emitting an error and not reconnecting) if the buffer reaches 100 MB without seeing a valid, complete EventSource line. Client can be configured by passing a custom `maxBufferSize`, but ideally the server would emit smaller chunks and/or use newlines more frequently.
BREAKING CHANGE: Library now requires node.js version 22.12 or higher. Technically speaking it still supports older engines, but it will not be guaranteed going forward since Node 20 is out of LTS. BREAKING CHANGE: Drop separate CommonJS variant. Node.js 22.12 and higher transparently allows `require(esm)`, so this shouldn't be a breaking change for most people - but marking it as such in an abudance of caution. Dropping the CommonJS variant lowers the risk of the "dual package hazard".
BREAKING CHANGE: Support for Chrome < 84, Safari < 15, Firefox < 105 and Edge < 84 dropped. This also includes any other javascript environment that does not support private fields, methods and accessors.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
A pending reconnection timer would keep the Node.js (and Bun) event loop alive, preventing the process from exiting while waiting to reconnect. Calling `unref()` on the timer opts out of that. The call is feature-checked: browsers and Deno return a numeric timer handle with no `unref()`, so it is only invoked when available.
…ener The `onopen`/`onmessage`/`onerror` property handlers were previously stored separately and invoked directly, before `dispatchEvent()`. That made them always fire before any listener added via `addEventListener()`, regardless of the order in which they were registered. Per the HTML spec, an event handler IDL attribute fires in the order it was set, relative to other event listeners. The setters now register/unregister the handler via `addEventListener()`/`removeEventListener()` and the manual invocations have been removed, so dispatch order now follows the spec. BREAKING CHANGE: When both an `on*` property handler and an `addEventListener()` listener are registered for the same event, they now fire in registration order rather than always firing the `on*` handler first. Code that relied on the old ordering may observe a different sequence of callbacks.
rexxars
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGE: The client will now fail the connection (emitting an error and not reconnecting) if the buffer reaches 100 MB without seeing a valid, complete EventSource line. Client can be configured by passing a custom
maxBufferSize, but ideally the server would emit smaller chunks and/or use newlines more frequently.BREAKING CHANGE: Library now requires node.js version 22.12 or higher. Technically speaking it still supports older engines, but it will not be guaranteed going forward since Node 20 is out of LTS.
BREAKING CHANGE: Drop separate CommonJS variant. Node.js 22.12 and higher transparently allows
require(esm), so this shouldn't be a breaking change for most people - but marking it as such in an abundance of caution. Dropping the CommonJS variant lowers the risk of the "dual package hazard" and makes build tooling much easier.BREAKING CHANGE: Support for Chrome < 84, Safari < 15, Firefox < 105 and Edge < 84 dropped. This also includes any other javascript environment that does not support private fields, methods and accessors.