[next] A Show branch mounted by a held write reads its condition's old null when a binding probes isPending
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 50/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- javascript, typescript
Direzione di ricerca
Start in packages/signals/src/core/core.ts, reading frameRead(), recompute() and stagedScreen() alongside the proposed bornInto() helper. Reproduce the failure with the Vitest case described in the issue and add or inspect packages/web/test/show-held-branch-verdict-reader.spec.tsx. Done means the branch renders after the held write resolves without any caught error, for both unchanged and changed answers.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
A non-keyed <Show> branch that a held write mounts can get null from its narrowed accessor when one of its bindings also probes isPending on data the same write started loading. The binding throws (Cannot read properties of null (reading 'name')) into <Errored>. Since #3977, if the data then lands an unchanged value, the error stays and the branch never appears; on #3977's parent the landing re-runs the binding and the branch appears after a transient error. The branch should read the value that mounted it on every pass, with no error at all.
Reproduction
Vitest 5.0.3, jsdom 30.1.2, Vite 8.3.4 and @solidjs/vite-plugin 3.0.0-next.49:
// vitest.config.ts
import { defineConfig } from "vitest/config";
import solid from "@solidjs/vite-plugin";
export default defineConfig({
plugins: [solid()],
test: { environment: "jsdom" },
});
import { createSignal, createStore, Errored, flush, isPending, Loading, Show } from "solid-js";
import { render } from "@solidjs/web";
import { expect, test } from "vitest";
test("a non-keyed Show reveals its branch after dependent data resolves", async () => {
const [selected, setSelected] = createSignal<{ name: string } | null>(null);
const answer = Promise.withResolvers<{ count: number }>();
const host = document.createElement("div");
const errors: unknown[] = [];
const dispose = render(() => {
const [data] = createStore(() => (selected() ? answer.promise : { count: 0 }), { count: 0 });
return (
<Loading fallback="Loading">
<Errored
fallback={(error) => {
errors.push(error());
return <p>{String(error())}</p>;
}}
>
<Show when={selected()} fallback="Choose">
{(item) => (
<section
aria-label={item().name}
aria-busy={isPending(() => data.count) ? "true" : "false"}
>
{data.count}
</section>
)}
</Show>
</Errored>
</Loading>
);
}, host);
try {
flush();
expect(host.textContent).toBe("Choose");
setSelected({ name: "Example" });
flush();
answer.resolve({ count: 0 });
await answer.promise;
flush();
console.log({ errors: errors.map(String), final: host.innerHTML });
expect(host.querySelector("section")?.getAttribute("aria-label")).toBe("Example");
expect(host.textContent).toBe("0");
} finally {
dispose();
}
});
For each row, solid-js, @solidjs/signals and @solidjs/web were all built from that commit with their own build scripts.
| Build | Errors caught | Final DOM |
|---|---|---|
a497d4b1 (parent of #3977) |
1, transient | <section aria-label="Example" aria-busy="false">0</section> |
ad973ed5 (#3977) |
1 | <p>TypeError: Cannot read properties of null (reading 'name')</p> |
6232d18e (next) |
1 | <p>TypeError: Cannot read properties of null (reading 'name')</p> |
6232d18e with the possible fix below |
0 | <section aria-label="Example" aria-busy="false">0</section> |
Changes to the reproduction, run on the same four builds:
| Change | Parent, #3977 and next |
With the fix |
|---|---|---|
Replace the isPending attribute expression with the constant "false" |
No error; renders the section | Same |
Use keyed and read item.name |
No error; renders the section | Same |
Return { count: 0 } synchronously instead of the promise |
No error; renders the section | Same |
Resolve { count: 1 } instead |
One transient error, then renders 1 |
No error; renders 1 |
Cause
Both reads happen in packages/signals/src/core/core.ts. The section's attribute effect reads item() and probes isPending, so it is a verdict reader. The <Show> mounted it in the flush that then parked the write: its first pass published, and the seam put it in the transaction. When it runs again before the landing, it is not treated as part of that transaction's frame:
-
frameRead()exempts "a frame born into that transaction (held and never committed — created by one of its held passes, A29)" from the stale reader, but checks only for a node that has never run:(c._statusFlags & STATUS_UNINITIALIZED && c._config & CONFIG_HELD && txOf(c) === t)recompute()uses the same test before publishing a held render effect mainline. The effect has run once, so outside the transaction's flush it is published mainline and served the committed screen, where the<Show>condition is stillnull. -
Inside the transaction's flush,
stagedScreen()serves it the committed value too, because it is a verdict reader.
The committed screen has no place for this effect: its branch exists only in the transaction. Before #3977, the landing's reruns() re-ran the errored reader and hid the read. #3977 skips errored readers there:
if (r._flags & REACTIVE_DISPOSED || r._statusFlags & STATUS_ERROR) continue;
so the error remains unless a changed value notifies the binding again.
Expected
The non-keyed Show callback receives an Accessor<NonNullable<T>>; a branch should never read its condition's old null because a binding asks whether data is pending. The reproduction uses no latest() (#3948).
Possible fix
Treat a node as born into the transaction for as long as it has never been committed, not only before its first run. Tested on next at 6232d18e: a bornInto(c, t) helper is true for a node held by t that is uninitialized, or that a pass of t still awaiting its commit mounted (CONFIG_STAGED or uninitialized), directly or through the frame that pass mounted. It walks the owner chain and stops at a node of a parked frame (REACTIVE_ZOMBIE), which is on screen. recompute()'s mainline branch, frameRead()'s exemption and stagedScreen() use it. #3977's skip of errored readers stays as it is: the binding no longer errors.
With it, the reproduction and every change above run with no caught error, including the transient one. Results:
- A new regression test,
packages/web/test/show-held-branch-verdict-reader.spec.tsx(the reproduction, with an unchanged and a changed answer, asserting no caught errors), fails on unmodifiednextand passes with the fix. - The signals suite passes: 5,176 tests, plus 22 expected failures and 2 skipped, the same as unmodified
next. Its rules index (docs/RULES-INDEX.md) is regenerated for the new comments. - The
solid-jssuite passes 855 of 856 tests and the@solidjs/websuites 1,328 of 1,336, with 1 expected failure. In this checkout, without the fix, the same 8 tests also fail: they check built declarations and package manifests that were not built here.
The helper, in core.ts:
function bornInto(c: Computed<any>, t: Transaction): boolean {
if (!(c._config & CONFIG_HELD) || txOf(c) !== t) return false;
if (c._statusFlags & STATUS_UNINITIALIZED) return true;
for (let o: Owner | null = c; o !== null; o = o._parent) {
if ((o as Computed<any>)._flags & REACTIVE_ZOMBIE) return false;
if (o === c || !(o as Computed<any>)._fn) continue;
const pass = o as Computed<any>;
if (!(pass._config & CONFIG_HELD) || txOf(pass) !== t) return false;
if (pass._config & CONFIG_STAGED || pass._statusFlags & STATUS_UNINITIALIZED) return true;
}
return false;
}
(submitted by Claude Opus 5.5 on behalf of rvlzzr)
- Lingua principale
- TypeScript
- Stelle
- 36.1k
- Fork
- 1.1k
- Merge medio
- 10h 44m
- PR unite (30g)
- 339
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Ha un modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di solidjs/solid
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 28/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 45/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 58/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 62/100
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di solidjs/solid
Issue simili
-
documentation good first issue hacktoberfest help wanted
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
libredb/libredb-studio#1756 ·
I maintainer di solito rispondono entro 1 giorno
-
state:triage-needed
Difficoltà 2/5 1-3 ore Idoneità per principianti 62/100
I maintainer di solito rispondono entro 1 giorno
-
community first-timers-only good first issue hacktoberfest help wanted low hanging fruit up-for-grabs
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 72/100
lingdojo/kana-dojo#32365 · 1 commento · 5 reazioni ·
I maintainer di solito rispondono entro 1 giorno
-
friction
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
kentcdodds/kody#3706 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
copperheadhq/copperhead#378 ·
I maintainer di solito rispondono entro 1 giorno