upgrade: deep package upgrade for Solid 2.0#914
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:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🦋 Changeset detectedLatest commit: 67f7efc 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.changeset/deep-solid2-migration.md (1)
17-17: 💤 Low valueConsider rephrasing for clarity.
The phrase "required split compute/apply form" is slightly awkward. Consider one of these alternatives for better readability:
- "updated to use the split compute/apply form"
- "updated to the required split-form compute/apply pattern"
🤖 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 @.changeset/deep-solid2-migration.md at line 17, Update the changelog line that mentions `createEffect` to improve readability by rephrasing "updated to required split compute/apply form" to one of the clearer alternatives, e.g., "updated to use the split compute/apply form" or "updated to the required split-form compute/apply pattern"; locate the `createEffect` mention in the .changeset entry and replace the phrase accordingly.packages/deep/package.json (1)
64-69: ⚡ Quick winUpdate Solid.js beta pins in packages/deep/package.json (beta.13 vs latest beta)
- npm shows no stable
solid-js@2.0.0/@solidjs/web@2.0.0;solid-js“next” is2.0.0-beta.14(and@solidjs/webnext matches).2.0.0-beta.13still exists, but2.0.0-beta.14is the newer 2.0 beta—consider bumping bothsolid-jsand@solidjs/webto2.0.0-beta.14.- Security advisory found for
solid-jsonly applies to versions< 1.9.4(patched in1.9.4); none returned that implicates2.0.0-beta.13or@solidjs/web.🤖 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/deep/package.json` around lines 64 - 69, The package.json pins for the Solid.js packages are out of date; update both occurrences of "`@solidjs/web`" and "solid-js" (in "dependencies" and "devDependencies") from "2.0.0-beta.13" to "2.0.0-beta.14" so the project uses the newer 2.0 beta; ensure you change the string values for the keys "`@solidjs/web`" and "solid-js" in packages/deep/package.json to "2.0.0-beta.14".
🤖 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/deep/src/store-updates.ts`:
- Around line 173-180: Guard the use of the in-operator by checking that store
is a non-null object before testing $TRACK: change the condition to first verify
typeof store === "object" && store !== null (e.g. if (!(typeof store ===
"object" && store !== null && $TRACK in store))). Update the branch that
currently uses ($TRACK in store) so it only runs when that object guard passes,
leaving the DEV console.warn, init flag and the fallback return behavior
(init/[] closure) unchanged; use the same identifiers store and $TRACK to locate
the code.
In `@packages/deep/test/track.test.ts`:
- Around line 307-315: The test's compute callbacks are returning function
references instead of invoking them, so no dependencies are registered; update
the createEffect calls to invoke the functions: change the first createEffect
callback from () => fn to () => fn() (calling fn), and ensure the other compute
callback actually calls api.fn with the tracked value (use () => api.fn(a) or ()
=> { api.fn(a); } so it invokes api.fn rather than returning a reference).
Target the createEffect calls around sign.a and api.fn to make the effects
establish dependencies.
---
Nitpick comments:
In @.changeset/deep-solid2-migration.md:
- Line 17: Update the changelog line that mentions `createEffect` to improve
readability by rephrasing "updated to required split compute/apply form" to one
of the clearer alternatives, e.g., "updated to use the split compute/apply form"
or "updated to the required split-form compute/apply pattern"; locate the
`createEffect` mention in the .changeset entry and replace the phrase
accordingly.
In `@packages/deep/package.json`:
- Around line 64-69: The package.json pins for the Solid.js packages are out of
date; update both occurrences of "`@solidjs/web`" and "solid-js" (in
"dependencies" and "devDependencies") from "2.0.0-beta.13" to "2.0.0-beta.14" so
the project uses the newer 2.0 beta; ensure you change the string values for the
keys "`@solidjs/web`" and "solid-js" in packages/deep/package.json to
"2.0.0-beta.14".
🪄 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: 8a83ec67-f0f5-4d0f-92c9-45e8ef00b73a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/deep-solid2-migration.mdpackages/deep/README.mdpackages/deep/package.jsonpackages/deep/src/store-updates.tspackages/deep/src/track-deep.tspackages/deep/src/track-store.tspackages/deep/test/track.test.tspackages/deep/test/updates.test.ts
# Conflicts: # pnpm-lock.yaml
Dependencies**
solid-js@^2.0.0-beta.13and@solidjs/web@^2.0.0-beta.13createStore,reconcile,snapshot) moved fromsolid-js/store→solid-jsisServermoved fromsolid-js/web→@solidjs/webtrackStore— rewritten to fix two correctness bugs present in the old implementation:{ myStore: store }is not itself a store, so it's a no-op —pojo: falsebehaviour is preserved)trackDeep—traverseparameter typed asunknowninstead ofStore<T>, which is the correct type for a function that does runtime type-checking and recurses into untyped childrenstore-updatesunwrap→snapshot;createRoot→runWithOwner(null, ...)for detached lazy memoscreateStoreDelta→captureStoreUpdatesas Static,signal!,as anyon already-anynodes)Tests
batch()removed;flush()added after mutationsunwrap→snapshot; store setters converted to draft formreconcilecalls in tests replaced with direct draft mutations (beta.13 requires a key function argument, which these tests didn't need)README — all examples updated to split
createEffect, draft setters, and `snapshotSummary by CodeRabbit
Release Notes
Breaking Changes
@solid-primitives/deepto Solid.js v2.0 (beta.13). Updated peer dependency requirements and API patterns including import sources, store utilities, and effect usage.Documentation