Migrate Solid to 2.0 - #617
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
Performance comparison skippedA compatible base-source run was unavailable, so only current-source performance tests were executed. Commit 7812539 · full reports in the |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7812539. Configure here.
| registeredPlugins: Iterable<RegisteredPlugin>, | ||
| ): PluginStoreState => { | ||
| let mergedTheme: Required<Theme> = DEFAULT_THEME; | ||
| let mergedTheme = appliedTheme; |
There was a problem hiding this comment.
Theme sticks after plugin unregister
High Severity
createPluginStoreState now seeds theme merges from appliedTheme (the last applied result) instead of DEFAULT_THEME. Options still reset from defaults each recompute, but theme does not, so unregisterPlugin / dispose leave prior plugin theme fields such as hue or enabled in place.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7812539. Configure here.
| const element = store.contextMenuElement; | ||
| if (!element) return null; | ||
| return resolveSource(element); | ||
| }); |
There was a problem hiding this comment.
Context menu source memo is async
Medium Severity
contextMenuFilePath was migrated from createResource to a createMemo that returns the Promise from resolveSource, then read with fileInfo?.filePath and contextMenuHasFilePath outside any Loading / isPending handling. Open-file affordances and action filePath can stay wrong or suspend until the async graph settles.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 7812539. Configure here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 42 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/openstory/package.json">
<violation number="1" location="apps/openstory/package.json:25">
P3: vite-plugin-solid@3.0.0-next.27 is now an unused dependency. The Solid Vite plugin was moved to @solidjs/vite-plugin (imported in vite.config.ts), and nothing else references vite-plugin-solid. Remove the vite-plugin-solid devDependency instead of bumping it to keep package.json free of dead dependencies.</violation>
</file>
<file name="packages/react-grab/src/core/plugin-registry.ts">
<violation number="1" location="packages/react-grab/src/core/plugin-registry.ts:76">
P1: When a plugin is unregistered or replaced with a config omitting a theme field, `createPluginStoreState` starts from the already-applied merged theme. Rebuild each snapshot from `DEFAULT_THEME` so removed plugin theme values do not persist.</violation>
</file>
<file name="packages/react-grab/src/utils/create-anchored-dropdown.ts">
<violation number="1" location="packages/react-grab/src/utils/create-anchored-dropdown.ts:62">
P2: When a dropdown's contents change while it remains on the same edge, `anchorEdge` does not invalidate the effect, so `measure()` never refreshes the panel dimensions and viewport clamping can use stale values. Keep the edge-only dependency for listener setup, but measure again after panel content or size changes, such as with a container `ResizeObserver` or a separate measurement effect.</violation>
</file>
<file name="packages/react-grab/src/components/selection-label/completion-view.tsx">
<violation number="1" location="packages/react-grab/src/components/selection-label/completion-view.tsx:24">
P2: Because this callback now runs as a native element-property handler, Solid queues the fade and overlay updates without an explicit flush. The completion label can therefore remain visible for a scheduler turn after More options is clicked; import `flush` and call it immediately after `props.onClick()`.</violation>
</file>
<file name="packages/react-grab/src/core/store.ts">
<violation number="1" location="packages/react-grab/src/core/store.ts:203">
P2: The per-update `flush()` makes the store change visible before the related phase change, exposing transient inconsistent states that the removed `batch` previously hid. Preserve a single transaction for each multi-state action and flush only after both the store and `current` updates are applied.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| registeredPlugins: Iterable<RegisteredPlugin>, | ||
| ): PluginStoreState => { | ||
| let mergedTheme: Required<Theme> = DEFAULT_THEME; | ||
| let mergedTheme = appliedTheme; |
There was a problem hiding this comment.
P1: When a plugin is unregistered or replaced with a config omitting a theme field, createPluginStoreState starts from the already-applied merged theme. Rebuild each snapshot from DEFAULT_THEME so removed plugin theme values do not persist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/core/plugin-registry.ts, line 76:
<comment>When a plugin is unregistered or replaced with a config omitting a theme field, `createPluginStoreState` starts from the already-applied merged theme. Rebuild each snapshot from `DEFAULT_THEME` so removed plugin theme values do not persist.</comment>
<file context>
@@ -62,18 +61,19 @@ const createPluginRegistry = (initialOptions: SettableOptions = {}) => {
registeredPlugins: Iterable<RegisteredPlugin>,
): PluginStoreState => {
- let mergedTheme: Required<Theme> = DEFAULT_THEME;
+ let mergedTheme = appliedTheme;
let mergedOptions: OptionsState = { ...DEFAULT_OPTIONS, ...initialOptions };
const allContextMenuActions: ContextMenuAction[] = [];
</file context>
| // the toolbar tracks its bounding rect). Derive a stable | ||
| // "is the dropdown open" memo and key the effect off THAT. | ||
| const isAnchored = createMemo(() => anchorAccessor() !== null); | ||
| const anchorEdge = createMemo(() => anchorAccessor()?.edge ?? null); |
There was a problem hiding this comment.
P2: When a dropdown's contents change while it remains on the same edge, anchorEdge does not invalidate the effect, so measure() never refreshes the panel dimensions and viewport clamping can use stale values. Keep the edge-only dependency for listener setup, but measure again after panel content or size changes, such as with a container ResizeObserver or a separate measurement effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/utils/create-anchored-dropdown.ts, line 62:
<comment>When a dropdown's contents change while it remains on the same edge, `anchorEdge` does not invalidate the effect, so `measure()` never refreshes the panel dimensions and viewport clamping can use stale values. Keep the edge-only dependency for listener setup, but measure again after panel content or size changes, such as with a container `ResizeObserver` or a separate measurement effect.</comment>
<file context>
@@ -51,48 +51,46 @@ export const createAnchoredDropdown = (
// the toolbar tracks its bounding rect). Derive a stable
// "is the dropdown open" memo and key the effect off THAT.
- const isAnchored = createMemo(() => anchorAccessor() !== null);
+ const anchorEdge = createMemo(() => anchorAccessor()?.edge ?? null);
+ const isAnchored = createMemo(() => anchorEdge() !== null);
</file context>
| }; | ||
| element.onclick = (event) => { | ||
| event.stopImmediatePropagation(); | ||
| props.onClick(); |
There was a problem hiding this comment.
P2: Because this callback now runs as a native element-property handler, Solid queues the fade and overlay updates without an explicit flush. The completion label can therefore remain visible for a scheduler turn after More options is clicked; import flush and call it immediately after props.onClick().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/components/selection-label/completion-view.tsx, line 24:
<comment>Because this callback now runs as a native element-property handler, Solid queues the fade and overlay updates without an explicit flush. The completion label can therefore remain visible for a scheduler turn after More options is clicked; import `flush` and call it immediately after `props.onClick()`.</comment>
<file context>
@@ -15,8 +15,19 @@ interface MoreOptionsButtonProps {
+ };
+ element.onclick = (event) => {
+ event.stopImmediatePropagation();
+ props.onClick();
+ };
+ };
</file context>
| const [store, setDeferredStore] = createStore<GrabStore>(createInitialStore(input)); | ||
| const setStore = (updateStore: (draft: GrabStore) => void) => { | ||
| setDeferredStore(updateStore); | ||
| flush(); |
There was a problem hiding this comment.
P2: The per-update flush() makes the store change visible before the related phase change, exposing transient inconsistent states that the removed batch previously hid. Preserve a single transaction for each multi-state action and flush only after both the store and current updates are applied.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/core/store.ts, line 203:
<comment>The per-update `flush()` makes the store change visible before the related phase change, exposing transient inconsistent states that the removed `batch` previously hid. Preserve a single transaction for each multi-state action and flush only after both the store and `current` updates are applied.</comment>
<file context>
@@ -198,7 +197,11 @@ interface GrabActions {
+ const [store, setDeferredStore] = createStore<GrabStore>(createInitialStore(input));
+ const setStore = (updateStore: (draft: GrabStore) => void) => {
+ setDeferredStore(updateStore);
+ flush();
+ };
</file context>
| "typescript": "^6.0.3", | ||
| "vite": "npm:@voidzero-dev/vite-plus-core@^0.1.20", | ||
| "vite-plugin-solid": "^2.11.12" | ||
| "vite-plugin-solid": "3.0.0-next.27" |
There was a problem hiding this comment.
P3: vite-plugin-solid@3.0.0-next.27 is now an unused dependency. The Solid Vite plugin was moved to @solidjs/vite-plugin (imported in vite.config.ts), and nothing else references vite-plugin-solid. Remove the vite-plugin-solid devDependency instead of bumping it to keep package.json free of dead dependencies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/openstory/package.json, line 25:
<comment>vite-plugin-solid@3.0.0-next.27 is now an unused dependency. The Solid Vite plugin was moved to @solidjs/vite-plugin (imported in vite.config.ts), and nothing else references vite-plugin-solid. Remove the vite-plugin-solid devDependency instead of bumping it to keep package.json free of dead dependencies.</comment>
<file context>
@@ -13,14 +13,16 @@
"typescript": "^6.0.3",
"vite": "npm:@voidzero-dev/vite-plus-core@^0.1.20",
- "vite-plugin-solid": "^2.11.12"
+ "vite-plugin-solid": "3.0.0-next.27"
},
"optionalDependencies": {
</file context>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| return resolveSource(element); | ||
| }, | ||
| ); | ||
| const contextMenuFilePath = createMemo(() => { |
There was a problem hiding this comment.
contextMenuFilePath was changed from createResource to createMemo, so it now returns an unresolved Promise instead of the resolved ResolvedSource | null, breaking all file-path/line-number consumers.
1 participant
Summary
Why
Solid 2 changes package boundaries, lifecycle and effect APIs, store updates, event handling, and update scheduling. This migration updates the integration comprehensively so React Grab and its Openstory playground build and behave correctly on the new runtime.
Impact
React Grab and Openstory now use
solid-js@2.0.0-rc.0and@solidjs/web@2.0.0-rc.0. Openstory currently emits a peer warning becauseopenstory@0.1.0still declares Solid 1, but its build and typecheck pass.Validation
nr buildnr test— 893 functional browser tests passed, 27 skipped, one retry-only timing flake; 73 performance tests passednr lintnr typechecknr formatSummary by cubic
Migrates React Grab and Openstory to Solid 2.0 RC, updating package boundaries and effect APIs while preserving synchronous behavior and shadow‑DOM interactions. Old:
solid-js/web,vite-plugin-solid,onMount/onCleanup,on(); New:@solidjs/web,@solidjs/vite-plugin,onSettledwith returned cleanups, and direct effect signatures; addsflush()where needed.solid-js@2.0.0-rc.0,@solidjs/web@2.0.0-rc.0,babel-preset-solid@2.0.0-rc.0; switchrenderimport andjsxImportSourceto@solidjs/web; update Vite to@solidjs/vite-pluginand alias/dedupe@solidjs/web.onMount/onCleanupwithonSettledand returned cleanups; replaceon()watchers with directcreateEffectsignatures; useflush()in store and plugin registry to keep imperative flows synchronous.on:pointerdownwithonPointerDownor native listeners via refs wherestopImmediatePropagationis required; normalize ARIA booleans to"true"/"false".openstory@0.1.0declares Solid 1 (types and build pass).packages/react-grabdependencies changed; otherwise swapspackages/react-grab/srcfrom the base SHA for comparison.Written for commit 7812539. Summary will update on new commits.
Note
High Risk
Large, cross-cutting migration of the entire Solid overlay, core store, and scheduling on a 2.0 RC runtime; regressions in grab/freeze, menus, toolbar, and focus are plausible despite broad test coverage.
Overview
Migrates the overlay UI stack to Solid 2.0 RC (
solid-js,@solidjs/web,babel-preset-solid,@solidjs/vite-plugin/vite-plugin-solid3.x), moving DOM rendering and JSX types offsolid-js/weband updatingjsxImportSourceand Vite aliases/dedupe accordingly.Across
packages/react-graband Openstory, lifecycle and reactivity APIs are updated:onMount/onCleanup→onSettledwith returned cleanups,on()-wrappedcreateEffect→ Solid 2’s source/effect signatures,splitProps→omit, andMenuContext.Provider→MenuContext. Grab store and plugin registry updates now callflush()after deferred store writes so imperative activation/copy flows stay synchronous under Solid 2 scheduling.Shadow-DOM and a11y fixes for the new runtime include native
contextmenu/ pointer listeners (withstopImmediatePropagationwhere delegation is insufficient), stringaria-*values,readonlyinstead ofreadOnly, andclass/cninstead ofclassListwhere needed. Context menu source resolution dropscreateResourcefor a memo over asyncresolveSource.Perf CI skips swapping
packages/react-grab/srcto the PR base whenpackage.jsondependency fields differ (e.g. this Solid bump), and the diff step posts a “comparison skipped” summary when baseline artifacts are missing. E2e Solid source-location expectations are updated for moved toolbar source lines.Reviewed by Cursor Bugbot for commit 7812539. Bugbot is set up for automated code reviews on this repo. Configure here.