upgrade: websocket package upgrade for Solid 2.0#847
Merged
davedbase merged 14 commits intoMay 27, 2026
Conversation
🦋 Changeset detectedLatest commit: 04fa590 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 |
davedbase
changed the title
# Conflicts: # pnpm-lock.yaml
# Conflicts: # packages/memo/README.md # 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/websocket/README.md`:
- Line 11: The README.md heading "### Connection primitives" skips from h1 to
h3; change it to an h2 by replacing "### Connection primitives" with "##
Connection primitives" so headings increment properly and follow markdown best
practices; update the line in packages/websocket/README.md where the heading
appears.
🪄 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: af849d6b-5f5a-4a5c-99a1-2d1f197b0814
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.changeset/websocket-solid-2-async.mdpackages/websocket/CHANGELOG.mdpackages/websocket/README.mdpackages/websocket/package.jsonpackages/websocket/src/index.tspackages/websocket/test/index.test.ts
atk
approved these changes
May 27, 2026
atk
left a comment
atk
left a comment
Member
There was a problem hiding this comment.
I would prefer to make the examples more succinct, so the tool tips don't get too large.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/websocket/src/index.ts (1)
307-317: 💤 Low valueSolid 2.0 supports
createMemowith async iterables; the async-generator semantics are aligned—focus on TS typing for the cast.Solid 2.0’s async-data model explicitly supports async computations (including Async Iterables) inside
createMemo, where consuming the resulting accessor suspends until ready and then settles (Loading/isPending semantics). The only remaining question in this snippet is whetheras Accessor<U>is required for TypeScript inference; ifcreateMemo’s overloads already inferAccessor<U>from the yielded values, the cast can be removed or narrowed accordingly.🤖 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/websocket/src/index.ts` around lines 307 - 317, The return is force-cast to Accessor<U> unnecessarily; update createWSData to let TypeScript infer the accessor type (or explicitly type the createMemo call) instead of using "as Accessor<U>". Replace the cast by either removing "as Accessor<U>" so createMemo infers the AsyncIterable<U> (ensure transform: WSDataOptions<T,U> returns U) or by adding an explicit generic to createMemo like createMemo<U>(async function* () { for await (const msg of wsMessageIterable<T>(ws)) yield (transform ? transform(msg) : (msg as unknown as U)); }) so the resulting value is strongly typed as Accessor<U> without a forced cast; reference symbols: createWSData, createMemo, wsMessageIterable, WSDataOptions, transform, Accessor.packages/websocket/test/index.test.ts (2)
92-103: ⚡ Quick winPin “latest wins in a single flush” behavior.
This test only checks sequential updates with a flush between them. It doesn’t lock the documented burst semantics where multiple messages in one flush should expose only the latest value.
Suggested test addition
it("reflects the latest received message", () => createRoot(dispose => { const ws = createWS("ws://localhost:5000"); const message = createWSMessage<string>(ws); vi.advanceTimersByTime(20); // wait for open ws.dispatchEvent(new MessageEvent("message", { data: "hello" })); flush(); expect(message()).toBe("hello"); ws.dispatchEvent(new MessageEvent("message", { data: "world" })); flush(); expect(message()).toBe("world"); + + // same-flush burst: latest wins + ws.dispatchEvent(new MessageEvent("message", { data: "one" })); + ws.dispatchEvent(new MessageEvent("message", { data: "two" })); + flush(); + expect(message()).toBe("two"); dispose(); }));🤖 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/websocket/test/index.test.ts` around lines 92 - 103, The test should be extended to assert "latest wins in a single flush" by dispatching multiple MessageEvent messages on the same WebSocket before calling flush and then verifying createWSMessage's accessor returns only the last dispatched value; specifically, inside the createRoot block that uses createWS("ws://localhost:5000") and createWSMessage<string>(ws), after advancing timers for open (vi.advanceTimersByTime(20)) dispatch at least two MessageEvent("message", { data: "first" }) and MessageEvent("message", { data: "second" }) without calling flush between them, call flush once and expect message() to be "second", then dispatch another MessageEvent and flush to assert subsequent updates still work; this pins the documented burst semantics for createWSMessage/message().
208-215: ⚡ Quick winAssert suspension before first message explicitly.
Line 208’s test name claims pre-first-message suspension, but the current body only proves post-message resolution.
Suggested assertion tightening
it("suspends until the first message arrives", async () => { await createRoot(async dispose => { const ws = makeWS("ws://localhost:5000"); vi.advanceTimersByTime(20); const data = createWSData<string>(ws); + expect(() => data()).toThrow(); ws.dispatchEvent(new MessageEvent("message", { data: "hello" })); await resolve(() => data()); expect(data()).toBe("hello"); dispose(); }); });🤖 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/websocket/test/index.test.ts` around lines 208 - 215, The test currently only verifies post-message resolution; add an explicit assertion that the selector suspends before the first message by attempting to resolve createWSData<string>(ws) prior to ws.dispatchEvent and asserting that this resolve does not complete (e.g., it remains pending or times out) until you dispatch the MessageEvent; then dispatch the message and assert resolve(() => data()) completes and data() equals "hello". Target the test's createRoot block and the symbols makeWS, createWSData, resolve, and data() when adding the pre-dispatch suspension check.
🤖 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/websocket/stories/websocket.stories.tsx`:
- Around line 202-203: Update the story text to remove the word "planned" and
reflect that wsMessageIterable is available: edit the sentence referencing
createWSMessage<T>(ws) so it still explains that it returns a reactive
Accessor<T | undefined> with the most recently received message and replace "use
the planned `wsMessageIterable` when every message must be processed" with
wording that indicates `wsMessageIterable` is part of the current API (for
example, "use `wsMessageIterable` when every message must be processed").
---
Nitpick comments:
In `@packages/websocket/src/index.ts`:
- Around line 307-317: The return is force-cast to Accessor<U> unnecessarily;
update createWSData to let TypeScript infer the accessor type (or explicitly
type the createMemo call) instead of using "as Accessor<U>". Replace the cast by
either removing "as Accessor<U>" so createMemo infers the AsyncIterable<U>
(ensure transform: WSDataOptions<T,U> returns U) or by adding an explicit
generic to createMemo like createMemo<U>(async function* () { for await (const
msg of wsMessageIterable<T>(ws)) yield (transform ? transform(msg) : (msg as
unknown as U)); }) so the resulting value is strongly typed as Accessor<U>
without a forced cast; reference symbols: createWSData, createMemo,
wsMessageIterable, WSDataOptions, transform, Accessor.
In `@packages/websocket/test/index.test.ts`:
- Around line 92-103: The test should be extended to assert "latest wins in a
single flush" by dispatching multiple MessageEvent messages on the same
WebSocket before calling flush and then verifying createWSMessage's accessor
returns only the last dispatched value; specifically, inside the createRoot
block that uses createWS("ws://localhost:5000") and createWSMessage<string>(ws),
after advancing timers for open (vi.advanceTimersByTime(20)) dispatch at least
two MessageEvent("message", { data: "first" }) and MessageEvent("message", {
data: "second" }) without calling flush between them, call flush once and expect
message() to be "second", then dispatch another MessageEvent and flush to assert
subsequent updates still work; this pins the documented burst semantics for
createWSMessage/message().
- Around line 208-215: The test currently only verifies post-message resolution;
add an explicit assertion that the selector suspends before the first message by
attempting to resolve createWSData<string>(ws) prior to ws.dispatchEvent and
asserting that this resolve does not complete (e.g., it remains pending or times
out) until you dispatch the MessageEvent; then dispatch the message and assert
resolve(() => data()) completes and data() equals "hello". Target the test's
createRoot block and the symbols makeWS, createWSData, resolve, and data() when
adding the pre-dispatch suspension check.
🪄 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: fbf695f1-889b-47f4-a8b5-e18cc7852b0a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/websocket/README.mdpackages/websocket/dev/index.tsxpackages/websocket/package.jsonpackages/websocket/src/index.tspackages/websocket/stories/websocket.stories.tsxpackages/websocket/test/index.test.tstest-results/.last-run.json
💤 Files with no reviewable changes (1)
- packages/websocket/dev/index.tsx
✅ Files skipped from review due to trivial changes (2)
- test-results/.last-run.json
- packages/websocket/README.md
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
2 participants
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.
Migrates the websocket primitive to
solid-js@^2.0.0-beta.7and introduces a new reactive message primitive.Breaking changes
solid-js@^2.0.0-beta.7createEffectdoc examples updated to the Solid 2.0 split form:createEffect(compute, effect)New:
createWSMessage<T>Reactive
Accessor<T | undefined>that holds the latest received WebSocket message. Cleans up its event listener on owner disposal viaonCleanup.createWSStatefixInternal signal now uses
{ ownedWrite: true }to suppress the Solid 2.0 dev-modeSIGNAL_WRITE_IN_OWNED_SCOPEwarning, which fires whenws.close()is called from inside a component or reactive scope.Planned (documented, not yet implemented)
wsMessageIterable<T>— bufferedAsyncIterablefor burst-safe message processingcreateWSData<T>— async memo over the iterable; suspends<Loading>until first messagecreateWSStore<T>— store driven by WS messages as draft-mutation patchesSummary by CodeRabbit
Release Notes
Breaking Changes
New Features
Documentation