[next] A Show branch mounted by a held write reads its condition's old null when a binding probes isPending
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 50/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- javascript, typescript
Research direction
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.
Written by the indexing model from the issue text.
Description
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)
- Dominant language
- TypeScript
- Stars
- 36.1k
- Forks
- 1.1k
- Avg merge
- 10h 44m
- Merged PRs (30d)
- 339
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from solidjs/solid
-
Difficulty 4/5 3-5 days Newbie friendliness 28/100
Maintainers usually reply within 1 day
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 58/100
Maintainers usually reply within 1 day
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 62/100
Maintainers usually reply within 1 day
Similar issues
-
A hung CI job runs for 6 hours before it is cancelledPossibly taken @Olanetsoft claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Olanetsoft/agent-lookout#110 ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
thedotmack/claude-mem#4757 ·
Maintainers usually reply within 1 day
-
🐛 bug 🗺️ MCP OAuth
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
LibreChat-AI/LibreChat#17077 ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 75/100