[next] A Show branch mounted by a held write reads its condition's old null when a binding probes isPending
Maintainer thường phản hồi trong vòng 1 ngày
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức phù hợp với người mới
- 50/100
- Loại issue
- Lỗi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức độ hoạt động
- Sôi nổi
- Công nghệ
- javascript, typescript
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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)
- Ngôn ngữ chính
- TypeScript
- Star
- 36.1k
- Fork
- 1.1k
- Merge trung bình
- 10 giờ 44 phút
- Pull request đã merge (30 ngày)
- 339
Chuẩn bị môi trường
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của solidjs/solid
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 28/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 45/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 58/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 5/5 Hơn một tuần Mức phù hợp với người mới 35/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 62/100
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của solidjs/solid
Issue tương tự
-
A hung CI job runs for 6 hours before it is cancelledCó thể đã có người làm @Olanetsoft đã nhận hôm nay. Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 78/100
Olanetsoft/agent-lookout#110 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 90/100
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
thedotmack/claude-mem#4757 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
🐛 bug 🗺️ MCP OAuth
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 62/100
LibreChat-AI/LibreChat#17077 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Độ khó 1/5 Dưới một giờ Mức phù hợp với người mới 75/100