upgrade: selection package upgrade for Solid 2.0#895
Conversation
🦋 Changeset detectedLatest commit: 6a4cc10 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 |
|
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/selection/test/index.test.tsx`:
- Around line 103-105: Remove the conditional guards that allow tests to pass
when selection() is null: in the contentEditable tests replace the "if
(selection()[0] !== null) { expect(selection()).toEqual([div, 0, 6]); }" style
checks with deterministic assertions — e.g., first assert
expect(selection()[0]).not.toBeNull() (or expect(selection()).toHaveLength(3) as
appropriate) and then assert expect(selection()).toEqual([div, 0, 6]); do the
same for the other block (the assertions around lines 119-124) so the test fails
if selection() is unexpectedly null instead of skipping the core assertion.
- Around line 23-37: The test bodies call unmount() and dispose() only on the
success path, which leaks DOM/root state if an expect throws; wrap the test
logic inside a try/finally so that unmount() (from renderTest()) and dispose()
(from createRoot callback) are always called even on assertion failures, i.e.,
locate the createRoot(...) callback and surrounding code that uses renderTest(),
input, flush(), selection() and ensure the finally block calls unmount() and
dispose(); apply the same try/finally pattern to every test case (including the
ones spanning lines 40-52, 55-69, 72-84, 87-109, 112-128).
In `@packages/selection/test/server.test.ts`:
- Around line 8-9: The test currently calls setSelection with the initial tuple
which makes the no-op setter assertion weak; change the setter call in the test
to pass a distinct, non-default tuple (e.g., [1, 2, 3] or another clear
sentinel) via setSelection(...) and then assert that selection() still equals
the original [null, NaN, NaN]; update the lines using setSelection and the
subsequent expect to use the distinct input while keeping the expected value the
original tuple, referencing the setSelection and selection functions.
🪄 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: ed495f96-23ae-4705-bcb7-01fef76759ea
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/selection-solid2-migration.mdpackages/selection/README.mdpackages/selection/package.jsonpackages/selection/src/index.tspackages/selection/test/index.test.tsxpackages/selection/test/server.test.tspackages/selection/tsconfig.jsonpackages/selection/vitest.config.ts
src/index.ts`
isServerimport moved fromsolid-js/web→@solidjs/webINTERNAL_OPTIONSimported from@solid-primitives/utilsand applied to both internal signals sosetSelectioncan be called from reactive scopescreateEffect(event listener setup with no reactive deps) was removed — listeners now register directly withonCleanupcreateEffect(DOM apply) converted to the Solid 2.0 split pattern:createEffect(compute, apply)package.jsonsolid-jsbumped to2.0.0-beta.10(peer + dev)@solidjs/web: 2.0.0-beta.10added (peer + dev)@solid-primitives/utils: workspace:^added as a runtime dependency@babel/coreandbabel-preset-solid@2.0.0-beta.10added as devDepsvitestscript points to a localvitest.config.tsvitest.config.ts(new)babel-preset-solidwithmoduleName: "@solidjs/web", mirroring the virtual package's approach to work around the vite-plugin-solid / Solid 2.0 incompatibilitytest/index.test.tsxrenderimport from@solidjs/web;flushimported fromsolid-jscreateRoot, usingdispatchEvent+flush()in place of the oldawait createEffect(async () => {...})patterntest/server.test.ts(new) — SSR no-op verification.changeset/selection-solid2-migration.md— major bump changesetSummary by CodeRabbit
Release Notes
New Features
@solidjs/webv2.0.Bug Fixes
Documentation