upgrade: sse package upgrade for Solid 2.0#844
Conversation
🦋 Changeset detectedLatest commit: 699fca7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
# Conflicts: # packages/sse/README.md # packages/sse/src/sse.ts # packages/sse/test/index.test.ts # pnpm-lock.yaml
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
# Conflicts: # pnpm-lock.yaml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/sse/test/index.test.ts (1)
246-266: ⚡ Quick winAssert pending state immediately after reconnect paths.
Line 246 and Line 269 claim data resets to pending, but the tests only assert eventual new data. Add an immediate throw assertion after
reconnect()/setUrl(...)to prevent stale-data regressions from slipping through.✅ Suggested assertion additions
reconnect(); flush(); + expect(() => data()).toThrow(); // Old source closed, new source opened expect(source()).not.toBe(first); @@ setUrl("https://example.com/v2/events"); flush(); + expect(() => data()).toThrow(); // Old source closed, new source opened for v2 expect(source()).not.toBe(first);Also applies to: 269-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sse/test/index.test.ts` around lines 246 - 266, After calling reconnect() or setUrl(...) in the createSSE test, add an immediate assertion that data() is in the pending/reset state (i.e., the observable returned by createSSE should indicate "pending" right after the reconnection is initiated) before advancing timers or simulating messages; update both the reconnect() test and the setUrl(...) test to assert data() equals the pending/reset value immediately after calling reconnect() or setUrl(...) and before any vi.advanceTimersByTime/flush/simulateMessage calls so stale-data regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/sse/README.md`:
- Line 16: The TOC entry for createSSEStream uses the wrong fragment
(`#createssesstream`); update the link target to the correct anchor for the header
(use `#createssestream`) so the `createSSEStream` TOC link points to the actual
section; search for the TOC line referencing `createSSEStream` and replace the
fragment in the markdown link.
In `@packages/sse/vitest.config.ts`:
- Around line 18-29: The alias entries for "solid-js/web" and "`@solidjs/web`" use
hard-coded pnpm store paths; update both alias values to use module-relative
entry paths instead (server.js vs web.js) so resolution doesn't depend on pnpm
internals: replace the long ".pnpm/…/@solidjs/web/dist/..." strings with
relative module paths like "../../node_modules/@solidjs/web/dist/server.js" when
testSSR is true and "../../node_modules/@solidjs/web/dist/web.js" when false
(apply the same change for both "solid-js/web" and "`@solidjs/web`" alias
definitions in vitest.config.ts).
---
Nitpick comments:
In `@packages/sse/test/index.test.ts`:
- Around line 246-266: After calling reconnect() or setUrl(...) in the createSSE
test, add an immediate assertion that data() is in the pending/reset state
(i.e., the observable returned by createSSE should indicate "pending" right
after the reconnection is initiated) before advancing timers or simulating
messages; update both the reconnect() test and the setUrl(...) test to assert
data() equals the pending/reset value immediately after calling reconnect() or
setUrl(...) and before any vi.advanceTimersByTime/flush/simulateMessage calls so
stale-data regressions are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a5fdceb-fb6a-407d-a0ee-e1706ed8f1aa
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.changeset/sse-solid2-async-reactivity.mdpackages/sse/README.mdpackages/sse/package.jsonpackages/sse/src/index.tspackages/sse/src/sse.tspackages/sse/test/index.test.tspackages/sse/test/server.test.tspackages/sse/test/worker.test.tspackages/sse/vitest.config.ts
1 participant
Summary by CodeRabbit
Breaking Changes
pendinganderrorproperties from SSE return valuedataaccessor now throwsNotReadyErrorduring loading instead of returning undefineddata()accessorNew Features
makeSSEAsyncIterableto wrap SSE endpoints as async iterablescreateSSEStreamas a minimal reactive SSE alternativeDocumentation
<Loading>and<Errored>components