Skip to content

fix: useLoader+Suspense crash, duck-typed Material/Object3D attach, createResource for renderer init#55

Merged
bigmistqke merged 23 commits into
solidjs-community:nextfrom
bigmistqke:fixes
May 27, 2026
Merged

fix: useLoader+Suspense crash, duck-typed Material/Object3D attach, createResource for renderer init#55
bigmistqke merged 23 commits into
solidjs-community:nextfrom
bigmistqke:fixes

Conversation

@bigmistqke
Copy link
Copy Markdown
Contributor

@bigmistqke bigmistqke commented May 26, 2026

Three independently-valuable fixes that surfaced while working on the WebGPU + construction-firewall PRs but didn't belong in either's scope.

1. useLoader + <Suspense> crashed with "Hooks can only be used within the Canvas component"

The bug pattern (minimal repro):

function TexturedCube() {
  const texture = useLoader(THREE.TextureLoader, "/img.png")
  return <T.Mesh><T.MeshBasicMaterial map={texture()} /></T.Mesh>
}
<Canvas><Suspense><TexturedCube /></Suspense></Canvas>

Once the resource resolved, useThree() inside TexturedCube threw — the component was being re-instantiated in an owner without solid-three's context providers.

Root cause: createThree passed children to useSceneGraph via:

useSceneGraph(context.scene, mergeProps(props, { get children() { return c() }}))

mergeProps builds a resolveSources fallback chain for each key. When c() transiently returned undefined (which happened while the reactive chain re-evaluated through a <Suspense>), the chain fell through to the user's raw <Canvas> JSX children getter and invoked it — spawning a fresh Suspense from scratch, in an owner outside solid-three's Provider tree.

Fix: useSceneGraph only reads children (and an optional onUpdate Canvas never passes). No reason to merge with the full Canvas props surface. Pass a plain object literal with just the override; no fallback chain to the user's children getter.

-  useSceneGraph(context.scene, mergeProps(props, {
-    get children() { return c() }
-  }))
+  useSceneGraph(context.scene, { get children() { return c() } })

Tests:

  • tests/core/use-loader-suspense.test.tsx — new regression: useLoader inside no-fallback <Suspense>, verifies the user component is instantiated exactly once and that the mesh ends up in the scene after the resource resolves. Plus a second test exercising the explicit-fallback case (named "fallback" mesh swaps to named "content" mesh on resolve).
  • tests/core/hooks.test.tsx — re-enables three previously commented-out useLoader integration tests (single URL, record of URLs, onBeforeLoad option), all using <Suspense fallback={null}>.

2. Foreign Material / Object3D instances not recognized by attach logic

applySceneGraph used instanceof Material, instanceof Object3D, etc. to decide auto-attach slots. That fails when a class comes from a separate module instance of three — most notably MeshBasicNodeMaterial from three/webgpu doesn't share class identity with the Material base imported from three. Three.js itself uses duck-typed is* flags (isMaterial, isObject3D, isBufferGeometry, isFog) as the cross-version contract.

Fix:

  • Adds isMaterial, isObject3D, isBufferGeometry, isFog helpers in utils.ts next to the existing isWebXRManager / isWebGLShadowMap.
  • Replaces instanceof checks in src/props.ts with the new helpers. three.js imports there become type-only.

Test: tests/core/renderer.test.tsx includes a previously-failing repro "attaches a foreign Material (duck-typed isMaterial: true)" that now passes.

3. createResource instead of async createEffect for renderer init

The previous pattern was an async createEffect with closure-mutation and a hand-rolled cancelled flag for race protection:

let glInitialized = false
createEffect(async () => {
  const renderer = gl()
  glInitialized = false
  let cancelled = false
  onCleanup(() => { cancelled = true })
  const init = getPendingInit(renderer)
  if (init) { /* pre-size canvas */; await init() }
  if (!cancelled) glInitialized = true
})

createResource is the idiomatic primitive:

  • Source = () => gl(). Cancellation on swap is built in — no manual cancelled flag.
  • Fetcher returns sync true for renderers without init() → resource is "ready" immediately; or a Promise that resolves once init() finishes (WebGPU) → "pending" until then.
  • render() gates on rendererReady.state !== "ready" instead of a closure boolean.

Canvas pre-sizing (workaround for WebGPU's depth-buffer dimension lock — see pmndrs/react-three-fiber#3651) is unchanged.

No behavior change observable to tests; all 10 init-touching tests pass.

Test plan

  • Existing tests pass (pnpm test)
  • tsc --noEmit clean
  • New tests:
    • 2 in use-loader-suspense.test.tsx (no-fallback regression + explicit-fallback content swap)
    • 3 in hooks.test.tsx (single URL, record, onBeforeLoad — previously commented)
    • 1 in renderer.test.tsx (foreign material, previously a failing repro)
  • Manual smoke test in browser — recommended for the createResource refactor since jsdom uses a fake renderer and doesn't exercise the actual async init path.

`useLoader` resources read into a `<T.* />` prop inside `<Suspense>` (without
an explicit `fallback`) used to throw "S3: Hooks can only be used within the
Canvas component!" once the resource resolved. Reproduces with the user's
minimal pattern:

```tsx
function TexturedCube() {
  const texture = useLoader(THREE.TextureLoader, "…")
  return <T.Mesh><T.MeshBasicMaterial map={texture()} /></T.Mesh>
}
<Canvas><Suspense><TexturedCube /></Suspense></Canvas>
```

## What was happening

createThree wrapped the user's children in `mergeProps`:

```ts
useSceneGraph(context.scene, mergeProps(props, {
  get children() { return c() }
}))
```

`mergeProps` builds a `sources = [overrideGetter, canvasPropsGetter]` chain
for the `children` key and resolves via `resolveSources`, which returns the
first NON-undefined source value. When `c()` returned `undefined` (which
happened transiently while the reactive chain re-evaluated through a
nested `<Suspense>`), `resolveSources` fell through to the user's raw
`<Canvas>` JSX `get children()` accessor and invoked it — spawning a fresh
`Suspense` component from scratch in an owner that didn't include the
canvas's context providers. That second Suspense's `TexturedCube` then
ran `useThree()` and threw.

## The fix

`useSceneGraph` only reads `children` (and an optional `onUpdate` Canvas
never passes). There's no reason to merge with the full Canvas props
surface. Pass a clean object with just the override; no fallback chain to
the original `<Canvas>` children getter.

## Test

Adds tests/core/use-loader-suspense.test.tsx covering the exact pattern.
Verified the test fails on HEAD~1 and passes after the fix.
The three useLoader hook tests were commented out — they exercise patterns
that the recently-fixed Suspense+useSceneGraph interaction (see prior
commit) now handles correctly. Re-enabling them lands real regression
coverage for the user-facing pattern:

  function Component() {
    const resource = useLoader(...)
    return <Show when={resource()}>{r => <Entity from={r()} /></Show>
  }
  <Suspense fallback={null}><Component /></Suspense>

Adjustments to make them pass:
- `Primitive` doesn't exist in solid-three — use `Entity from={...}`.
- Loader-extension callback is now passed as `{ onBeforeLoad }` on the
  options object, not as a third positional argument.
- The "array of URLs" test was rewritten to use a Record. solid-three's
  `LoadInput` is `LoaderUrl | Record<string, LoaderUrl>` — array-of-URLs
  isn't supported (the test was carried over from a different API). The
  Record variant exercises the same multi-load path through
  `awaitMapObject`.
…fter resolve

The original regression test only verified no "Hooks can only be used"
error fired and that the component was instantiated exactly once. Beefs
the file up with:

1. The original no-fallback test now also captures the canvas's scene via
   `useThree()`, asserts no meshes are present while the resource is
   pending, and asserts exactly one mesh is present after resolve.

2. New test for explicit-fallback Suspense: asserts a named `fallback`
   mesh is in the scene while pending and gone after resolve, with the
   named `content` mesh swapping in. Verifies the full Suspense
   fallback↔content swap through solid-three's scene graph.
Resolves the f4d310b failing repro. `applySceneGraph` was using
`instanceof Material`, `instanceof Object3D`, etc. to decide auto-attach
slots. That fails when a class comes from a different module instance of
three — most notably `MeshBasicNodeMaterial` from `three/webgpu` won't
share class identity with the `Material` base imported from `three`.

Three.js itself uses duck-typed `is*` markers (`isMaterial`,
`isObject3D`, `isBufferGeometry`, `isFog`) as the cross-version contract.
Mirror that here.

- Adds `isMaterial`, `isObject3D`, `isBufferGeometry`, `isFog` helpers in
  utils.ts (alongside `isWebXRManager` / `isWebGLShadowMap`).
- Replaces `instanceof Material/Object3D/BufferGeometry/Fog` in
  `src/props.ts` with the new helpers. The three.js imports in props.ts
  become type-only since nothing else needs them at runtime.
- The pre-existing failing test "attaches a foreign Material (duck-typed
  isMaterial: true)" now passes.
The previous async-createEffect-with-closure-flag pattern for awaiting
`WebGPURenderer.init()` had several rough edges:
- Mutated a closure `glInitialized` boolean from inside an async effect.
- Hand-rolled cancellation via a `let cancelled` flag + onCleanup.
- No built-in error path if `init()` rejected.

`createResource` is the idiomatic primitive for "tracked async load":
- Source = `() => gl()`. When the renderer swaps, the in-flight fetch is
  cancelled automatically — no manual `cancelled` flag.
- Fetcher returns synchronous `true` for renderers without `init()`
  (resource is `"ready"` immediately, render loop can render) or a
  Promise that resolves once `renderer.init()` finishes (resource is
  `"pending"` until then).
- `render()` gates on `rendererReady.state !== "ready"` instead of a
  closure boolean.

Canvas pre-sizing (workaround for WebGPU's depth-buffer dimension lock)
is unchanged — still happens before awaiting init.

No behavior change observable to tests; all 10 init-touching tests pass.
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 26, 2026

commit: 56d8d04

Resource exposed an `attach` prop in its types and docs, but it was
stripped by useProps' splitProps and never written to the resource's
meta. When the resource was rendered as a JSX child, the parent's
applySceneGraph fell through to the auto-attach fallbacks
(material/geometry/fog/object3d) and ignored `attach` entirely — so
the documented `<Resource ... attach="map" />` pattern silently
attached nothing (or errored for non-Object3D resources).

Wrap the resolved value with meta(..., { props }) so the surrounding
scene graph sees attach (and any other meta-driven props) on the
loaded resource.
@bigmistqke
Copy link
Copy Markdown
Contributor Author

bigmistqke commented May 26, 2026

Follow-up commits on this branch (push pending):

  • 3c488ce refactor(utils): consolidate isRenderer (was isRendererInstance in create-three.tsx) and isWritable (was in props.ts) into utils.ts alongside the other is* guards.

  • dbbe477 test: add tests/core/api-coverage.test.tsx covering previously-untested public API:

    • createEntity (constructor + args propagation)
    • meta / getMeta / hasMeta
    • useProps (reactive prop application to an existing instance)
    • load standalone (single URL + record of URLs)
    • Resource (Object3D auto-add + render-fn)
    • CursorRaycaster + CenterRaycaster (NDC mapping)
  • 4ac1ac7 fix(Resource): surfaced while writing the coverage tests — <Resource attach="..." /> was a no-op. Resource called useProps(resource, rest), which split attach into local without ever writing it to the resource's meta. When the resource was rendered as a JSX child, the parent's applySceneGraph fell through to the auto-attach fallbacks and ignored the prop. Fix: wrap the resolved value with meta(value, { props }) so the parent reads attach off the child's meta. Includes a TDD regression test.

bigmistqke added 14 commits May 26, 2026 20:30
WebGPURenderer's XRManager has no `setAnimationLoop` — the loop driver
lives on the renderer itself in both WebGL and WebGPU builds. The old
`isWebXRManager` guard treated WebGPU as 'not XR-capable' and silently
skipped session wiring, so entering an XR session with a WebGPURenderer
left `xr.enabled` false and frames continued running off
`requestAnimationFrame` instead of the headset.

Replaces `isWebXRManager` with `canDriveXR(gl)`: xr is event-target-shaped
AND `setAnimationLoop` exists on the renderer. Session handlers now call
`gl.setAnimationLoop(...)`, which works for both renderer kinds.
`ref={canvas!}` compiles as `ref={null}` — Solid's babel transform only
recognises a bare `Identifier` as a ref-setter binding, and `canvas!` is
a `TSNonNullExpression` so the value of `canvas` (still `null`) gets
passed in. The refs were never bound, leaving `context.canvas` null
until event setup tried `addEventListener` on it.

Surfaced by running the test suite in real Chromium — jsdom hid it under
a different effect-flush order. Also the exact anti-pattern called out
in CONTRIBUTING.md.
Upgrades vitest to 4.x and adds @vitest/browser + Playwright + Chromium
so we can run tests against a real browser (catches the class of bug
where jsdom's leniency masks real DOM ordering / Solid compile issues —
e.g. the canvas-ref bug fixed in the previous commit).

`vitest.config.ts` (jsdom) stays as the default; `vitest.browser.config.ts`
runs the same suite in Chromium. Both now scope `include` to `tests/` so
stale .claude/worktrees copies don't get picked up. Adds .vitest-attachments
and tests __screenshots__ output to .gitignore.
…ract suite

Single `vitest.config.ts` with two projects:
- `browser` — full tests/ in Chromium (default, catches DOM-timing bugs)
- `jsdom` — tests/jsdom/, covers the public `src/testing/` API external
  consumers depend on so it can't bitrot silently

`pnpm test` runs both. Removes the temporary `vitest.browser.config.ts`.
Browser-mode vitest needs the Chromium binary downloaded into Playwright's
cache. Without this step the runner finds no executable and crashes
during browser-test launch.
- Task 6: createResource is gone in Solid 2.x; use createSignal(() => Promise) + isPending
- Task 12: next-solid-2 already has Resource shape using omit/Loading; rebase port onto it
- Task 14: Suspense → Loading in test imports
- Add top-of-plan audit table of Solid 1.x → 2.x API differences
`src/testing/` no longer ships a jsdom shim — it mounts a real `<canvas>` to
`document.body`, lets the browser provide WebGL/getBoundingClientRect/events,
and exposes a `cleanup()` helper that disposes the renderer and removes the
canvas after each test. Browsers cap concurrent WebGL contexts (~16 in
Chromium); without cleanup the suite crashes the page mid-run.

Also:
- vitest config: single browser project, `fileParallelism: false` (real
  WebGL contexts don't survive parallel files), SwiftShader software WebGL
  to bypass Chromium's GPU-process context cap.
- Event tests: replaced `new Event(type) + Object.defineProperty(offsetX/Y)`
  with real `MouseEvent`/`PointerEvent`/`WheelEvent` constructors —
  `offsetX/Y` is now computed by the browser from the canvas bounding rect.
- Two construction-firewall tests (`should recreate the renderer when
  tuple[0] changes shape` and its disposal counterpart) are now `it.skip` —
  WebGL doesn't allow rebinding a fresh context to a canvas after
  `forceContextLoss`, so the recreate-default-WebGLRenderer-on-same-canvas
  scenario can't work in real WebGL. The factory branch is still covered.
- Removed `tests/jsdom/` contract suite and the jsdom dev dependency.
…enderer

WebGL binds a context to a canvas for life — `forceContextLoss()` permanently
disables that canvas's context, so a freshly-constructed WebGLRenderer can't
acquire a new one on the same element. Previously this hung in real browsers
(the tests had to be skipped); the mock context hid it.

Fix:
- `create-three.tsx`: track the live canvas in a signal. When the gl memo
  re-runs in the default branch and the previous renderer was ours, create
  a fresh `<canvas>` (preserving dimensions + style), `replaceChild` it
  into the DOM, and construct the new WebGLRenderer against it. The memo
  reads via `untrack` to avoid looping on its own write.
- `create-events.ts`: wrap each `addEventListener` call in a `createEffect`
  via `bindToLiveCanvas` helper. Listeners re-bind to the new canvas (and
  are cleaned off the old one) when the swap happens.
- `context.canvas` becomes a reactive getter onto the live canvas.
- Re-enabled the two previously-skipped `renderer.test.tsx` recreation tests.
…warn

Drops the `gl={[ctorArgs, props]}` tuple form in favor of a flat
`gl={Partial<Props & WebGLRendererParameters>}` (r3f's shape). solid-three
internally splits constructor-only keys (alpha, antialias, depth, …) from
instance-writable ones at construction time.

Why: the tuple's tuple[0]-recreates-renderer behavior was a lie under real
WebGL — `getContext` is idempotent per canvas, so ctor args can never
actually change after first construction. The previous fix (replaceChild
the canvas + reactive `context.canvas` + per-listener createEffect) was
~80 lines of plumbing for a feature nobody uses. We now bake ctor args
once and warn (once) if the user reactively changes a ctor-only key, so
the limitation is visible instead of silent.

Changes:
- `canvas.tsx`: `gl` accepts `Partial<Props<WebGLRenderer> & WebGLRendererParameters>`.
- `create-three.tsx`: drop `glConstructorArgs` memo, `liveCanvas` signal,
  canvas-swap logic, `context.canvas` getter. gl memo's default branch
  splits keys via a `WEBGL_CTOR_KEYS` whitelist, reads ctor args
  untracked. New createEffect warns once on reactive ctor-arg change.
- `create-events.ts`: revert `bindToLiveCanvas` reactivity — listeners
  bind once to `context.canvas` again.
- tests: drop the 3 tuple-form tests; add flat-form smoke test, reactive
  instance-prop test, and warn-on-ctor-arg-change test.
README: update Canvas `gl` prop description (flat object of ctor args +
instance props, warn on reactive ctor-arg change), and the TypeScript
interface signature.

playground/api/canvas/usage.tsx: switch the demo from the factory form
to the flat form to show the idiomatic shape.
JSDoc still described the dropped `[ctorArgs, properties]` tuple. Rewrites
the bullet for the flat-object form (ctor args baked, instance props
reactive, warn on reactive ctor-key change).
…launch

The vitest browser instance type has no `launch` key — Playwright launch
args go to the `playwright()` factory's `launchOptions`. TypeScript
caught it in CI. Same SwiftShader flags, correct shape.
@bigmistqke bigmistqke merged commit 8348eb8 into solidjs-community:next May 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant