From e0c30bc43c752bd2eb1ac2547f6d3978ad8a691c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 21:03:33 +0000 Subject: [PATCH 1/3] fix(signals): createOwner's dispose works when called detached (#4444) `queueMicrotask(o.dispose)` on an owner from `createOwner()` threw "Cannot read properties of undefined (reading '_flags')". Root cause: since e16371faf every owner's `dispose` is the shared `disposeRootSelf`, which reads its owner from `this`; a detached call leaves `this` undefined and `disposeChildren(undefined)` reads `._flags`. The `createRenderEffect(..., { schedule: true })` in the report is incidental: a bare owner throws the same way, with or without effects, synchronously or from any task. Called as a method (`o.dispose()`), disposal before, during and after a scheduled first run already worked. The exported `createOwner` now binds `dispose` to its owner (as `createRoot`'s dispose is, #4097). Internal owners (mapArray rows, boundaries, reveal) keep the shared method and pay no allocation; no per-pass or flush code is touched. No F-/D- rule governs the call shape of a disposer; the disposal it performs is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2 --- .changeset/owner-detached-dispose-4444.md | 5 + packages/signals/src/core/index.ts | 1 + packages/signals/src/core/owner.ts | 16 +++ packages/signals/src/index.ts | 2 +- .../tests/owner-detached-dispose-4444.test.ts | 126 ++++++++++++++++++ 5 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 .changeset/owner-detached-dispose-4444.md create mode 100644 packages/signals/tests/owner-detached-dispose-4444.test.ts diff --git a/.changeset/owner-detached-dispose-4444.md b/.changeset/owner-detached-dispose-4444.md new file mode 100644 index 0000000000..7266da7f36 --- /dev/null +++ b/.changeset/owner-detached-dispose-4444.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +`createOwner()`'s `dispose` can be passed around as a plain function again — `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — instead of throwing `TypeError: Cannot read properties of undefined (reading '_flags')` (#4444). The method was a shared function reading its owner from `this`, which a detached call leaves undefined; the exported `createOwner` now binds it, as `createRoot`'s dispose already was. Internal owners (`mapArray` rows, boundaries) keep the shared method, so they pay no allocation. diff --git a/packages/signals/src/core/index.ts b/packages/signals/src/core/index.ts index 7c0e40fd4c..28a1d16481 100644 --- a/packages/signals/src/core/index.ts +++ b/packages/signals/src/core/index.ts @@ -27,6 +27,7 @@ export { } from "./external.js"; export { createOwner, + createPublicOwner, createRoot, dispose, getNextChildId, diff --git a/packages/signals/src/core/owner.ts b/packages/signals/src/core/owner.ts index 626cda0f10..0d3fd39699 100644 --- a/packages/signals/src/core/owner.ts +++ b/packages/signals/src/core/owner.ts @@ -444,6 +444,22 @@ export function createOwner(options?: { id?: string; transparent?: boolean }) { return owner; } +/** + * The exported `createOwner` (see `createOwner` above): its `dispose` is bound + * to the owner, so it can be handed around as a plain function — + * `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — as `createRoot`'s + * dispose can (#4444). Internal owners (a `mapArray` row, a boundary) keep the + * shared `this`-reading method: they are only ever disposed as `o.dispose()`, + * and a bound function per row would be an allocation per row. + * + * @internal + */ +export function createPublicOwner(options?: { id?: string; transparent?: boolean }) { + const owner = createOwner(options); + owner.dispose = disposeRootSelf.bind(owner); + return owner; +} + /** * Creates a reactive root — an owner scope with its own `dispose()`. A root * created inside an existing owner is owned by it and is disposed when the diff --git a/packages/signals/src/index.ts b/packages/signals/src/index.ts index c91b6cef24..3586fc91c5 100644 --- a/packages/signals/src/index.ts +++ b/packages/signals/src/index.ts @@ -6,7 +6,7 @@ export { NotReadyError, TimeoutError, createContext, - createOwner, + createPublicOwner as createOwner, createRoot, runWithOwner, flush, diff --git a/packages/signals/tests/owner-detached-dispose-4444.test.ts b/packages/signals/tests/owner-detached-dispose-4444.test.ts new file mode 100644 index 0000000000..2f6fe6c8c6 --- /dev/null +++ b/packages/signals/tests/owner-detached-dispose-4444.test.ts @@ -0,0 +1,126 @@ +/** + * #4444: `createOwner()`'s `dispose` is handed around as a plain function + * (`queueMicrotask(o.dispose)`, `setTimeout(o.dispose)`, an event listener) + * and threw `TypeError: Cannot read properties of undefined (reading + * '_flags')`: the method was a shared function that read its owner from + * `this`, which a detached call leaves undefined. Nothing about the effect + * in the report mattered — a bare owner threw the same way. + * + * No F-/D- rule governs this: it is the call shape of the owner's disposer, + * not a reactive semantic. The disposal it performs is unchanged: everything + * under the owner dies, cleanups run, and an effect whose first run is still + * scheduled never runs. + */ +import { describe, expect, it } from "vitest"; +import { + createEffect, + createOwner, + createRenderEffect, + flush, + isDisposed, + onCleanup, + runWithOwner +} from "../src/index.js"; + +const macrotask = () => new Promise(r => setTimeout(r, 0)); + +describe("createOwner dispose called detached (#4444)", () => { + it("the issue's shape: a scheduled render effect, owner disposed from a microtask", async () => { + const ran: string[] = []; + const o = createOwner(); + runWithOwner(o, () => { + createRenderEffect( + () => {}, + () => { + ran.push("effect"); + return () => ran.push("cleanup"); + }, + { schedule: true } + ); + queueMicrotask(o.dispose); + }); + // The flush microtask was queued at creation, ahead of the dispose, so the + // first run lands and the dispose then tears it down. + await macrotask(); + expect(isDisposed(o)).toBe(true); + expect(ran).toEqual(["effect", "cleanup"]); + }); + + it("a detached dispose disposes, with `self` defaulting to true", () => { + const o = createOwner(); + let cleaned = 0; + runWithOwner(o, () => onCleanup(() => cleaned++)); + const dispose = o.dispose; + dispose(); + expect(isDisposed(o)).toBe(true); + expect(cleaned).toBe(1); + }); + + it("a detached dispose(false) keeps the owner and disposes its contents", () => { + const o = createOwner(); + let cleaned = 0; + runWithOwner(o, () => onCleanup(() => cleaned++)); + const dispose = o.dispose; + dispose(false); + expect(isDisposed(o)).toBe(false); + expect(cleaned).toBe(1); + }); + + for (const kind of ["createRenderEffect", "createEffect"] as const) { + for (const schedule of [true, false]) { + const make = kind === "createRenderEffect" ? createRenderEffect : createEffect; + const label = `${kind}${schedule ? " { schedule: true }" : ""}`; + + it(`${label}: detached dispose before the first scheduled run — effects never run`, async () => { + const ran: string[] = []; + const o = createOwner(); + runWithOwner(o, () => { + for (let i = 0; i < 3; i++) + make( + () => i, + v => { + ran.push(`e${v}`); + }, + schedule ? { schedule: true } : undefined + ); + }); + const dispose = o.dispose; + dispose(); + flush(); + await macrotask(); + expect(isDisposed(o)).toBe(true); + // A render effect without `schedule` runs its effect at creation; the + // rest defer to the flush, which the disposal empties first. + expect(ran).toEqual(kind === "createRenderEffect" && !schedule ? ["e0", "e1", "e2"] : []); + }); + + for (const when of ["microtask", "macrotask"] as const) { + it(`${label}: detached dispose from a ${when}`, async () => { + const ran: string[] = []; + const o = createOwner(); + runWithOwner(o, () => { + for (let i = 0; i < 3; i++) + make( + () => i, + v => { + ran.push(`e${v}`); + return () => ran.push(`c${v}`); + }, + schedule ? { schedule: true } : undefined + ); + }); + if (when === "microtask") queueMicrotask(o.dispose); + else setTimeout(o.dispose, 0); + await macrotask(); + await macrotask(); + flush(); + expect(isDisposed(o)).toBe(true); + // Each effect ran once (the creation flush precedes the dispose) and + // was cleaned up by the dispose; none runs again after it. + expect(ran.filter(s => s[0] === "e").sort()).toEqual(["e0", "e1", "e2"]); + expect(ran.filter(s => s[0] === "c").sort()).toEqual(["c0", "c1", "c2"]); + }); + } + } + } +}); From eea75cf7b578312e3374ef2d8888e2952f59391b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 21:33:33 +0000 Subject: [PATCH 2/3] fix(signals,solid,web): the runtimes' own owners skip createOwner's bound dispose (#4444) NoHydration, Portal and a frame's fill called the exported createOwner, so every page carried the bind wrapper (+51 to +63 B minified) for a dispose they only ever call as o.dispose() or through the parent. They now take the unbound owner from an @internal signals export (createScopeOwner), re-exported through solid-js/internal. Apps that never call createOwner are back to 0 B; the router pages still pay it, since @solidjs/router calls the public createOwner. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2 --- .changeset/owner-detached-dispose-4444.md | 4 +++- packages/signals/src/index.ts | 9 +++++++++ packages/solid/src/client/hydration.ts | 4 ++-- packages/solid/src/internal.ts | 3 ++- packages/web/frames/src/client.ts | 10 +++++----- packages/web/src/index.ts | 5 ++--- 6 files changed, 23 insertions(+), 12 deletions(-) diff --git a/.changeset/owner-detached-dispose-4444.md b/.changeset/owner-detached-dispose-4444.md index 7266da7f36..c44766a96c 100644 --- a/.changeset/owner-detached-dispose-4444.md +++ b/.changeset/owner-detached-dispose-4444.md @@ -1,5 +1,7 @@ --- "@solidjs/signals": patch +"solid-js": patch +"@solidjs/web": patch --- -`createOwner()`'s `dispose` can be passed around as a plain function again — `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — instead of throwing `TypeError: Cannot read properties of undefined (reading '_flags')` (#4444). The method was a shared function reading its owner from `this`, which a detached call leaves undefined; the exported `createOwner` now binds it, as `createRoot`'s dispose already was. Internal owners (`mapArray` rows, boundaries) keep the shared method, so they pay no allocation. +`createOwner()`'s `dispose` can be passed around as a plain function again — `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — instead of throwing `TypeError: Cannot read properties of undefined (reading '_flags')` (#4444). The method was a shared function reading its owner from `this`, which a detached call leaves undefined; the exported `createOwner` now binds it, as `createRoot`'s dispose already was. Internal owners (`mapArray` rows, boundaries) keep the shared method, so they pay no allocation. `NoHydration`, `Portal` and a frame's fill use the unbound owner too, through `solid-js/internal`, so apps that never call `createOwner` pay nothing. diff --git a/packages/signals/src/index.ts b/packages/signals/src/index.ts index 3586fc91c5..9d0144dc0c 100644 --- a/packages/signals/src/index.ts +++ b/packages/signals/src/index.ts @@ -38,6 +38,15 @@ export { * outside dev builds. */ export { setConsoleFooter } from "./core/index.js"; +/** + * @internal `createOwner` as the runtimes in this repo use it (`NoHydration`, + * `Portal`, a frame's fill): its `dispose` is the shared `this`-reading + * method, not the bound one the exported `createOwner` gives (#4444). They + * dispose it as `o.dispose()` or with its parent, so apps that never call + * `createOwner` pay no bind. Client runtime only: the server has its own + * owners. + */ +export { createOwner as createScopeOwner } from "./core/index.js"; /** @internal The flush round counter (scheduler.ts): hydration's served pass tells its own round from a later one. */ export { clock } from "./core/index.js"; /** @internal Dev duplicate-copy check: solid-js marks components and context reads. */ diff --git a/packages/solid/src/client/hydration.ts b/packages/solid/src/client/hydration.ts index adea48aa36..86a9cb1aa7 100644 --- a/packages/solid/src/client/hydration.ts +++ b/packages/solid/src/client/hydration.ts @@ -39,7 +39,7 @@ import { type StoreOptions, type StoreSetter, type RevealOrder, - createOwner, + createScopeOwner, createRoot as coreRoot, getContext, setContext, @@ -3120,7 +3120,7 @@ function hydratedCreateLoadingBoundary( * ``` */ export function NoHydration(props: { children: SolidElement }): SolidElement { - const o = createOwner(); + const o = createScopeOwner(); return runWithOwner(o, () => { setContext(NoHydrateContext, true); // Claiming, not the bare flag: a rejected fragment's fresh render diff --git a/packages/solid/src/internal.ts b/packages/solid/src/internal.ts index 9a14d48a5f..b6456aea31 100644 --- a/packages/solid/src/internal.ts +++ b/packages/solid/src/internal.ts @@ -61,7 +61,8 @@ export { SOURCE_PROXY, SOURCE_MEMO, SOURCE_MERGE, - type SourceKind + type SourceKind, + createScopeOwner } from "@solidjs/signals"; /** diff --git a/packages/web/frames/src/client.ts b/packages/web/frames/src/client.ts index 3719206b29..3b210660bb 100644 --- a/packages/web/frames/src/client.ts +++ b/packages/web/frames/src/client.ts @@ -19,7 +19,6 @@ import { OBSERVE, createMemo, - createOwner, createRenderEffect, createSignal, getOwner, @@ -71,7 +70,8 @@ import { setWriteHook, signalName, sharedConfig, - storeName + storeName, + createScopeOwner } from "solid-js/internal"; // The container tier (DR-2 case 3): server projections cross the border as @@ -766,8 +766,8 @@ function slotsFor(props: Record, scope?: ClaimScope) { // hydrate pass consumes no id from the adopting component's counter // (a keyed sibling after the frame keys the same whether a fill // mounted at t=0 or after a hold; the claim window has its own id). - const fillOwner = streamInvoke ? createOwner() : null; - const owner = fillOwner || createOwner({ transparent: true }); + const fillOwner = streamInvoke ? createScopeOwner() : null; + const owner = fillOwner || createScopeOwner({ transparent: true }); fillScopes.set(key, owner); ctx.onCleanup(() => { if (fillScopes.get(key) === owner) fillScopes.delete(key); @@ -825,7 +825,7 @@ function slotsFor(props: Record, scope?: ClaimScope) { // re-call with existing nodes must render for real — claiming would // no-op its inserts and silently drop whatever the re-call // displaced, e.g. moved-out {$frame} region ranges (#547). - const claim = adopted && runWithOwner(owner, () => createOwner({ id: prefix })); + const claim = adopted && runWithOwner(owner, () => createScopeOwner({ id: prefix })); const value = adopted ? claimRender(prefix, claim, ctx.existing, evaluate, scope) : fillOwner diff --git a/packages/web/src/index.ts b/packages/web/src/index.ts index 06c4f93ebe..412f2a2fb6 100644 --- a/packages/web/src/index.ts +++ b/packages/web/src/index.ts @@ -13,7 +13,6 @@ import { import { createComponent, createMemo, - createOwner, createRoot, createSignal, getOwner, @@ -28,7 +27,7 @@ import { type Setter, type DynamicOptions } from "solid-js"; -import { dynamicCore, sharedConfig } from "solid-js/internal"; +import { createScopeOwner, dynamicCore, sharedConfig } from "solid-js/internal"; import type { JSX } from "../jsx/jsx.js"; export * from "./client.js"; @@ -135,7 +134,7 @@ export function Portal(props: { mount?: Element; children: JSX.Element }): JSX.E // child-id counter and shift every hydration id after the portal. Both // sides consume exactly one slot from the parent instead — the owner // here, `getNextChildId` in the server Portal. - return runWithOwner(createOwner(), () => portalImpl(props)) as unknown as JSX.Element; + return runWithOwner(createScopeOwner(), () => portalImpl(props)) as unknown as JSX.Element; } function portalImpl(props: { mount?: Element; children: JSX.Element }): JSX.Element { From a4ab913bbda78d50d1ac8ab85d748f4306e904db Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 22:37:32 +0000 Subject: [PATCH 3/3] docs: user-facing createOwner doc, list createScopeOwner in internal.ts, scope the changeset Review nits on #4508, comment-only: the public `createOwner` shows a user-facing doc comment instead of the maintainer note, the `solid-js/internal` header lists `createScopeOwner`, and the changeset no longer says "again" (a detached `getOwner().dispose` on other owner kinds still throws; out of #4444's scope). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2 --- .changeset/owner-detached-dispose-4444.md | 2 +- packages/signals/src/core/owner.ts | 19 +++++++++++-------- packages/solid/src/internal.ts | 5 +++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/.changeset/owner-detached-dispose-4444.md b/.changeset/owner-detached-dispose-4444.md index c44766a96c..f978c952c8 100644 --- a/.changeset/owner-detached-dispose-4444.md +++ b/.changeset/owner-detached-dispose-4444.md @@ -4,4 +4,4 @@ "@solidjs/web": patch --- -`createOwner()`'s `dispose` can be passed around as a plain function again — `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — instead of throwing `TypeError: Cannot read properties of undefined (reading '_flags')` (#4444). The method was a shared function reading its owner from `this`, which a detached call leaves undefined; the exported `createOwner` now binds it, as `createRoot`'s dispose already was. Internal owners (`mapArray` rows, boundaries) keep the shared method, so they pay no allocation. `NoHydration`, `Portal` and a frame's fill use the unbound owner too, through `solid-js/internal`, so apps that never call `createOwner` pay nothing. +`createOwner()`'s `dispose` can be passed around as a plain function — `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — instead of throwing `TypeError: Cannot read properties of undefined (reading '_flags')` (#4444). The method was a shared function reading its owner from `this`, which a detached call leaves undefined; the exported `createOwner` now binds it, as `createRoot`'s dispose already was. Internal owners (`mapArray` rows, boundaries) keep the shared method, so they pay no allocation. `NoHydration`, `Portal` and a frame's fill use the unbound owner too, through `solid-js/internal`, so apps that never call `createOwner` pay nothing. diff --git a/packages/signals/src/core/owner.ts b/packages/signals/src/core/owner.ts index 0d3fd39699..db370d5d2a 100644 --- a/packages/signals/src/core/owner.ts +++ b/packages/signals/src/core/owner.ts @@ -444,15 +444,18 @@ export function createOwner(options?: { id?: string; transparent?: boolean }) { return owner; } +// The exported `createOwner` (the internal one is above): its `dispose` is +// bound to the owner, so it can be handed around as a plain function, as +// `createRoot`'s dispose can (#4444). Internal owners (a `mapArray` row, a +// boundary) keep the shared `this`-reading method: they are only ever disposed +// as `o.dispose()`, and a bound function per row would be an allocation per row. /** - * The exported `createOwner` (see `createOwner` above): its `dispose` is bound - * to the owner, so it can be handed around as a plain function — - * `queueMicrotask(o.dispose)`, `setTimeout(o.dispose)` — as `createRoot`'s - * dispose can (#4444). Internal owners (a `mapArray` row, a boundary) keep the - * shared `this`-reading method: they are only ever disposed as `o.dispose()`, - * and a bound function per row would be an allocation per row. - * - * @internal + * Creates a fresh owner attached as a child of the current owner (or as a + * detached root if there is none), to group cleanups and context under one + * scope. Its `dispose()` tears that scope down and is bound to the owner, so it + * can be passed around as a plain function: `queueMicrotask(owner.dispose)`. + * To host a reactive scope outside a component, prefer `createRoot()`; to + * re-enter a captured owner, use `runWithOwner()`. */ export function createPublicOwner(options?: { id?: string; transparent?: boolean }) { const owner = createOwner(options); diff --git a/packages/solid/src/internal.ts b/packages/solid/src/internal.ts index b6456aea31..b49b8cdfcf 100644 --- a/packages/solid/src/internal.ts +++ b/packages/solid/src/internal.ts @@ -27,6 +27,11 @@ * build boundaries without the components, and so are `sharedConfig` * (the hydration/SSR coordination object) and the `$DEVCOMP` brand. * + * Also re-exported from `@solidjs/signals`: `createScopeOwner`, the owner the + * runtimes create for their own scopes (`NoHydration`, `Portal`, a frame's + * fill). It keeps the shared `this`-reading `dispose`, where the public + * `createOwner` binds it per owner (#4444). + * * Not here: the container-trace materializer. It lives in its own entry, * `solid-js/internal/container-trace`, so the store engine it builds on can * be a lazy chunk — a re-export from this (eagerly imported) module would