Solidjs V2 Reviewer

Review Solid 2.0 code

Hunt the two prior-knowledge bug classes — React reflexes and Solid 1.x reflexes — plus 2.0-specific reactivity mistakes. Severity guide: 🔴 broken behavior, 🟡 dev-mode diagnostic / lost reactivity, 🔵 style drift.

Confirm the project is actually v2 first (solid-js major 2 in package.json / @solidjs/web in deps). Reviewing a 1.x project against this list produces garbage findings.

Pass 1 — greppable smells

Run these over the changed files; each hit needs a fix or a justification.

Solid 1.x-isms

React-isms

2.0-specific

Pass 2 — judgement checks (not greppable)

• Derive vs write-back: any effect whose apply phase sets reactive state is suspect — usually a memo/projection in disguise.
• Boundary ownership: isPending reads placed under the Loading boundary that owns the data read? Pending indicators outside can never fire.
• Mutation shape: server writes wrapped in action() with optimistic state and a final refresh() or until() acknowledgement? If later code assumes the refresh finished, it must await/yield refresh(); ignored fire-and-forget is valid only when completion is intentionally unobserved.
• Action call site: an action may be defined in a component, but is it invoked only from an imperative scope? A component-body/computation call is a transaction-starting write and throws in dev mode.
• Optimistic spinner off isPending: a "Saving…" indicator driven by isPending on data the same action just wrote optimistically normally stays hidden — not because the optimistic write masks it (optimistic writes are verdict-inert), but because a bare refresh() after the write is a quiet same-question re-ask. (Published rc.5 has a narrow held-landing one-frame pulse defect, which is another reason not to treat this as process state.) The flag belongs in the data (co-written pending: true or a separate createOptimistic(false)); if the reload itself should read pending, that needs an explicit affects(target) before the refresh().
• SSR setter writes: signal/store setters in server render are deprecated and warn; optimistic server setters are no-ops. Model incoming changes as async sources instead of pushing through setters.
• Granularity: selection/derived caches notifying whole collections → createProjection. Fixed-slot lists diffed with <For> → <Repeat>.
• Shallow-store writes: with { shallow: true }, are nested raw records mutated in place? That is inert; replace the root property/array slot by reference. When refreshes rebuild row objects, use consumer keying such as <For keyed={row => row.id}> if row DOM identity must survive.
• Reconcile model: omission means key "id", null means fully positional, and missing item keys fall back positionally. Shape mismatch at a nested array/object slot replaces it. Do not approve claims that standalone reconcile silently swaps a different root entity (it is strict), or the inverse claim that projection/derived-store returned roots cannot perform an authoritative swap. At a shallow boundary reconciliation compares records by reference rather than mutating their fields.
• Derived-store readiness: a broad catch around async reads in a derive can swallow NotReadyError, commit a partial value, and prevent SSR retry. Let readiness propagate; do not "stabilize" a projection by catching it.
• Patch-driver boundary: published rc.5 can drive eligible ordinary, projection, and optimistic store arrays; projection recomputes emit row/slot ops and optimistic structural edits emit in-flight row ops plus revert resync. Explicit custom-key lists still decline to the classic path, and shallow arrays support multiple consumers. Application code must not force admission or call patch APIs.
• Server-function wrapping: an outer wrapper around a function-level directive cannot guard HTTP dispatch, so validation belongs in the body. A module-level server file is deliberately different: the evaluated terminal wrapped function is registered whole and runs on HTTP + direct SSR. Do not report that rc.5 module-level wrapped exports are compile errors.
• Custom server-function fetch: does it forward init, preserve same-origin, return an unread response, and avoid replay after any response was received? Dropped signals break abort/live teardown; replay can duplicate mutations.
• Raw-object assumptions: platform/native host objects are raw by default; their slot reassignment is reactive but internal mutation is not. User class instances remain wrappable, and markRaw is not a public root API.
• Ownership: module-scope effects/roots intentional? Detached lifetime must be explicit (runWithOwner(null, ...)).
• Composable naming: a createX/useX prefix should match lifecycle, not React habit — createX makes a fresh instance owned by the caller, useX is a shared singleton or accesses an already-created thing (useContext). useX is not wrong by itself (singletons are legit); flag only a per-call instance named useX, or every composable defaulting to useX out of reflex.
• Layout lane: DOM-geometry reads (getBoundingClientRect/offset*) belong in a createRenderEffect (render lane), not in a ref callback (node may be pre-insert/pre-layout there). Beware the inverse "fix" too: moving a layout measure out of createRenderEffect into createEffect/a ref on the false theory that render effects read a disconnected node — they don't; the trigger is flush-scheduled and runs after insertion.
• Tests: flush() after writes; createRoot wrappers; resolve() for async settling.

Reporting

Report findings ordered by severity with file:line, the broken expectation (one line), and the concrete 2.0 fix. Note clean areas that were checked. For deep API verification during review, the solidjs-v2 skill's references cover signatures; installed typings in node_modules are the final word for a moving prerelease API.