new: async package for Solid 2.0#909
Open
atk wants to merge 14 commits into
Open
Conversation
|
|
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
davedbase
reviewed
May 22, 2026
feat: fromJSONStream - new primitive test: improve tests feat: makeRetrying WIP
feat: support node-based web streams
docs: first storybook stories, README finalized
docs: finish stories
docs: finish stories
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
packages/async/test/index.test.ts (1)
14-15: ⚡ Quick winClose test streams when data is exhausted.
Both helper streams return on
donewithoutcontroller.close(), so completion semantics (done: true) aren’t being exercised cleanly.Proposed fix
- if (done) return; + if (done) { + controller.close(); + return; + } @@ - if (done) return; + if (done) { + controller.close(); + return; + }Also applies to: 62-63
🤖 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/async/test/index.test.ts` around lines 14 - 15, The helper streams in the test are not properly closing when data is exhausted. In both locations where the code checks if done is true and returns early (around the controller.enqueue calls), add a controller.close() call before the return statement to ensure the stream completion semantics are properly exercised. This applies to both instances where this pattern appears in the file.
🤖 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/async/package.json`:
- Around line 4-5: In the package.json file, update the placeholder metadata
fields: replace the "description" field value from "A template primitive
example." with an accurate description of what the async package does, and
replace the "author" field value from "Your Name <you@youremail.com>" with the
actual author's name and email address. These fields will be published to npm,
so ensure they contain real, meaningful information.
- Around line 35-47: The exports field in package.json declares a
`@solid-primitives/source` export condition that points to ./src/index.ts, but the
files array only includes the dist directory. This creates a mismatch where
consumers trying to use the `@solid-primitives/source` export will fail because
the src directory is not published to npm. Fix this by choosing one of two
approaches: either add "src" to the files array to publish source files
alongside dist, or remove the `@solid-primitives/source` export condition from the
exports field to prevent consumers from attempting to resolve unpublished source
files.
In `@packages/async/README.md`:
- Around line 13-18: The anchor fragments in the README table of contents use
camelCase format (fromStream, fromJSONStream, createAggregated, etc.) but
standard markdown slug generation converts headings to lowercase. Update all the
anchor fragment links in the primitive index to use lowercase versions
(fromstream, fromjsonstream, makeabortable, createabortable, makeretrying,
createaggregated) so they correctly resolve to the actual heading slugs in the
document.
- Around line 64-66: The README documentation contains mismatches with the
actual implementation for the fromJSONStream API. First, correct the function
label from "fromStream" to "fromJSONStream" in the definition block that shows
the actual function signature. Second, update all documentation examples and
option descriptions to use the correct option name "autoAbort" instead of
"noAutoAbort", ensuring the semantics are clearly documented that autoAbort
controls whether streams are automatically aborted (with inverse logic to what
the docs currently show). Apply these corrections to all instances mentioned
including the definition block around line 64-66 and the additional locations at
lines 108-109 and 118.
In `@packages/async/src/index.ts`:
- Around line 229-231: The toArray function uses a falsy check that incorrectly
converts valid falsy values like 0, false, and empty strings into empty arrays,
losing data during aggregation. Replace the ternary condition in toArray to
explicitly check for null or undefined values only (using == null or equivalent
checks) instead of relying on truthiness, so that valid falsy values like 0,
false, and "" are wrapped in arrays rather than discarded. Apply the same fix to
the other instance mentioned at lines 256-257.
- Around line 171-173: The isIterable and isAsyncIterable functions use
Object.hasOwn to check for Symbol.iterator and Symbol.asyncIterator, but this
only detects own properties and misses symbols on the prototype chain. Arrays,
strings, and generators have these symbols inherited from their prototypes,
causing makeRetrying to incorrectly treat them as single values. Replace
Object.hasOwn with the in operator for both isIterable and isAsyncIterable,
since the in operator checks the entire prototype chain and will correctly
identify all standard iterables and async iterables.
- Around line 195-197: The `retries` variable is being declared outside the
generator function and captured in the closure, causing state to leak and be
shared across multiple invocations. Additionally, using the `||` operator with
`options.retries` treats `0` as falsy, preventing `retries: 0` from being a
valid option to disable retries. Move the `retries` variable declaration inside
the generator function so each invocation gets its own independent retry
counter, and change the assignment from `options.retries || 3` to use the
nullish coalescing operator `??` instead to properly handle `0` as a legitimate
value.
In `@packages/async/stories/createAbortable.stories.tsx`:
- Around line 74-76: The input element with placeholder "type for autosuggest"
is using the onChange event handler which only fires on blur in Solid.js,
causing delayed autosuggest updates. Replace the onChange handler with onInput
on this input element to ensure the setQuery callback fires immediately on every
keystroke, providing instant suggestion responsiveness.
In `@packages/async/stories/fromJSONStream.stories.tsx`:
- Line 33: The JSON stream chunking in the anonymous function has an off-by-one
error in the slice end index calculation. The slice call currently subtracts 1
from the end index with `(idx + 1) * sliceLength - 1`, which removes one
character from each chunk and corrupts the JSON parsing. Remove the `- 1` from
the end index so that the slice correctly includes all characters up to the
proper boundary at `(idx + 1) * sliceLength`.
In `@packages/async/stories/fromStream.stories.tsx`:
- Line 33: The slice call in the callback function (around the `source.slice`
expression) is incorrectly subtracting 1 from the end index parameter, which
causes character loss at chunk boundaries because JavaScript's slice method
already treats the end index as exclusive. Remove the `- 1` from the end
parameter calculation in the slice call so it becomes `(idx + 1) * sliceLength`
instead of `(idx + 1) * sliceLength - 1`.
In `@packages/async/stories/makeAbortable.stories.tsx`:
- Around line 75-77: The input element in the makeAbortable.stories.tsx file is
using the onChange event handler, which in SolidJS only fires when the input
loses focus, causing a lag in the auto-suggest behavior. Replace the onChange
prop with onInput on the input element to ensure the setQuery callback fires on
every keystroke, providing real-time suggestions as the user types.
In `@packages/async/test/index.test.ts`:
- Line 168: Remove the debug console.log(result) statement from the retry test
in packages/async/test/index.test.ts. This console.log call serves no purpose in
the test and creates unnecessary noise in the CI/test output. Simply delete the
line containing console.log(result) from the test file.
In `@packages/resource/README.md`:
- Around line 11-12: The migration tip in the README.md file has unmatched
backtick markup that breaks markdown rendering. The text solid-js@>=2.0.0 has a
closing backtick but is missing the opening backtick, so it should be wrapped as
`solid-js@>=2.0.0` instead of solid-js@>=2.0.0` to properly render as inline
code.
---
Nitpick comments:
In `@packages/async/test/index.test.ts`:
- Around line 14-15: The helper streams in the test are not properly closing
when data is exhausted. In both locations where the code checks if done is true
and returns early (around the controller.enqueue calls), add a
controller.close() call before the return statement to ensure the stream
completion semantics are properly exercised. This applies to both instances
where this pattern appears in the file.
🪄 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: 2c4b2be6-35f2-4925-951b-e93fb948c6fe
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
packages/async/LICENSEpackages/async/README.mdpackages/async/package.jsonpackages/async/src/index.tspackages/async/stories/createAbortable.stories.tsxpackages/async/stories/data.jsonpackages/async/stories/fromJSONStream.stories.tsxpackages/async/stories/fromStream.stories.tsxpackages/async/stories/makeAbortable.stories.tsxpackages/async/stories/makeAggregated.stories.tsxpackages/async/stories/makeRetrying.stories.tsxpackages/async/stories/tsconfig.jsonpackages/async/test/index.test.tspackages/async/test/server.test.tspackages/async/tsconfig.jsonpackages/resource/README.md
Deploying solid-primitives with
|
| Latest commit: |
c8287b3
|
| Status: | 🚫 Build failed. |
brenelz
reviewed
Jun 21, 2026
brenelz
reviewed
Jun 21, 2026
brenelz
approved these changes
Jun 21, 2026
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
3 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.
This should replace the resource package, featuring the following primitives:
fromStream/fromJSONStream: (small helper to aggregate web streams); maybe in the future, we can also support fromJSONpStream for progressive JSON or similar formatsmakeAbortable/createAbortable: the same as in the resource package, but chainable for actionsmakeRetryingmakeCache(-> will go into the fetch package)Summary by CodeRabbit
New Features
@solid-primitives/asyncpackage featuring stream utilities for incremental data processing, abort/signal management with timeout support, automatic retry mechanisms with configurable delays, and reactive value aggregation capabilities.Documentation