[BUGFIX] renderComponent: don't depend on globalThis.Element#21364
[BUGFIX] renderComponent: don't depend on globalThis.Element#21364alexkahndev wants to merge 2 commits intoemberjs:mainfrom
Conversation
`renderComponent` does `into instanceof Element` in two places to
discriminate `Cursor` from `Element` / `SimpleElement`. That works in a
browser (where `Element` is a built-in) and inside FastBoot (which
patches a simulated `Element` onto `globalThis`), but throws
`ReferenceError: Element is not defined` on a plain Node / Bun host
that calls `renderComponent` with a fresh `@simple-dom/document` node
as `into`.
The function already has an `intoTarget()` helper a few lines up that
classifies the same union without touching any global:
function intoTarget(into: IntoTarget): Cursor {
if ('element' in into) {
return into;
} else {
return { element: into as SimpleElement, nextSibling: null };
}
}
This commit aligns the two `into instanceof Element` outliers with that
helper:
* The first-render `innerHTML` clearing branch becomes a structural
check — `!('element' in into) && 'innerHTML' in into` — so it
fires only on a real DOM `Element` and skips on `SimpleElement`
(which has no `innerHTML` setter) and `Cursor`.
* The re-render `parentElement` extraction calls `intoTarget(into)`
directly instead of duplicating the discriminator. Drive-by fix:
the previous `(into as Cursor).element` cast on the else branch
was incorrect for `SimpleElement` input — `SimpleElement` has no
`.element` property — though that path is rare in practice.
Net effect: `renderComponent` works with any of the three `IntoTarget`
shapes in any host environment, with no dependency on
`globalThis.Element`.
Adds a regression test that calls `renderComponent` with
`globalThis.Element` set to `undefined` and asserts the render
completes.
| * | ||
| * Use a structural check instead of `into instanceof Element` so the | ||
| * renderer doesn't depend on the `Element` constructor being a | ||
| * global. Browsers always have it; Node / Bun servers running with a |
There was a problem hiding this comment.
You're likely going to need to simulate a browser env anyway
Which reminds me! This RFC for Deprecation targeting v8 may affect you
There was a problem hiding this comment.
Thanks for the RFC heads-up I really appreciate it. Our adapter is using all three of simple-dom + hasDOM: true + isInteractive: false, so this would have ambushed us hard in v8. Will plan the migration to a happy-dom-shaped pattern (matching vite-ember-ssr) before we do anything user-facing.
There was a problem hiding this comment.
well, it's not an accepted RFC yet. the community may revolt agaist this idea. haha
in general tho, I think all SSR/SSG environments should be no different from browser tho, becaues otherwise you infect all library code like a virus, special casing the existence or non-existence of various things everywhere (window, etc (fastboot was really bad at this, for example))
There was a problem hiding this comment.
Strong agreement on the underlying stance and worth saying that we've watched four other frameworks land in the same place from different angles, which is some signal that the principle is sound.
Quick tour through what AbsoluteJS sees across our existing adapters:
React's SSR is string-based (renderToReadableStream). The "is this SSR?" question is bounded to a single boundary in the user's mental model: effects don't run on the server. Render code is identical on both sides. Users essentially never write if (typeof window !== 'undefined') for anything outside an effect and even there, the convention is to put it inside useEffect/useLayoutEffect which only fires client-side. The split is in when things run, not what code runs.
Vue lands almost the same way string-based via renderToWebStream, lifecycle hooks like onMounted are client-only by contract. Vue's small improvement on the React shape is onServerPrefetch, an additive primitive ("do this on the server") rather than a defensive check ("am I on the server?"). Better posture against the virus problem because it inverts the polarity. Svelte moves the boundary to compile time entirely. The Svelte compiler emits components twice, once as a string-concatenating function (SSR), once as DOM-mutating code (client). User-written .svelte files are identical for both; the compiler picks the
codegen. From a user's perspective, "am I in SSR or browser?" basically doesn't exist there's nothing to ask. The cost is the build pipeline knows about both modes; the user doesn't.
Svelte moves the boundary to compile time entirely. The Svelte compiler emits each component twice once as a string-concatenating function (SSR mode), once as DOM-mutating code (client mode). User-written .svelte files are identical for both; the
compiler picks the codegen. From a user's perspective, "am I in SSR or browser?" basically doesn't exist there's nothing to ask.
Angular is the one that maps closest to where this RFC takes Ember.
@angular/platform-server wraps Domino internally, so when user code does inject(DOCUMENT) or reads el.nativeElement it gets a real Document / Element instance regardless of side. There is no isFastBoot / isInteractive flag visible to component authors. Angular's user code is server-agnostic because the platform owns the simulated DOM, not the user.
The common thread every framework that successfully kept the "this is SSR" virus out of library code did it by eliminating the runtime question, not by exposing a flag for it. React/Vue split it on lifecycle phases. Svelte splits it at compile time. Angular pretends the DOM is always there. None of them ask user code or addon code "are you
running on the server?" at runtime, and none of them have the FastBoot-style ecosystem fragmentation.
Ember post-RFC ends up on Angular's branch of that tree happy-dom (or any DOM) installs Element/Node/Document, the renderer always assumes interactive mode, and user/addon code stops needing to ask. From outside Ember, that looks like the framework finally adopting the same architectural answer the rest of the ecosystem converged on years ago just by removing the affordance to ask the question, rather than by adding a new abstraction layer.
The empirical evidence across React, Vue, Svelte, and Angular is consistent: the runtime "am I in SSR?" question is the source of the virus. Removing the question by whichever mechanism a framework can is what stops the spread. That seems like a strong technical case for the direction even outside the specifics of isInteractive / hasDOM.
There was a problem hiding this comment.
this is a fantastic summary, but can you post in on the RFC as a comment? <3
| this.assertStableRerender(); | ||
| } | ||
|
|
||
| '@test renderComponent does not depend on a global Element constructor'( |
There was a problem hiding this comment.
Does this test fail without your fix?
There was a problem hiding this comment.
Yes. Without the structural-check fix, the original into instanceof Element line throws once the test sets globalThis.Element = undefined:
- (globalThis as any).Element = undefined → TypeError: Right hand side of instanceof is not an object (the test's setup)
- delete globalThis.Element (production-equivalent path on a bare Node host) → ReferenceError: Element is not defined
Either path crashes the renderer before the render loop runs. With the fix, both paths short-circuit through the structural !('element' in into) && 'innerHTML' in into check and the render completes successfully. Test runs green on the patched renderer; reverting just the renderer hunk (keeping the test) reproduces the failure.
End-to-end Ember Phase 1: a .gts page with @glimmer/component + @Tracked SSRs to clean HTML through Bun + Elysia + ember-source 6.12. Three load-bearing pieces: src/build/buildEmberVendor.ts Builds stable vendor files at {buildDir}/ember/vendor/ for @ember/template-compiler, @ember/renderer, @glimmer/component, @glimmer/tracking, @embroider/macros. Uses a Bun.build resolver plugin to route every @ember/*/@glimmer/*/@simple-dom/* import to the absolute path inside node_modules/ember-source/dist/packages/ — works around a Bun resolver bug where the package's `"./*": "./dist/packages/*"` exports wildcard fails on @-prefixed subpaths (oven-sh/bun#30187 — see EMBER_BANDAID #1). @embroider/macros is virtualized via onResolve/onLoad to a small shim — its real index throws by design because it expects compile-time replacement (EMBER_BANDAID #2). src/build/compileEmber.ts Compiles .gjs/.gts/.ts entries: content-tag's `Preprocessor.process()` extracts <template> blocks and replaces them with `template(...)` calls + an @ember/template-compiler import; the JS half feeds through Bun.Transpiler (with experimentalDecorators: true to keep Glimmer's legacy @Tracked semantics — see EMBER_PLAN §0.1's stage-3 decorator note); the SSR side gets a full Bun.build pass with the same resolver plugin shape, producing a self-contained server bundle that exports renderToHTML(props). Client side is just the transpiled module — the framework's client bundle pass picks it up. src/ember/{pageHandler,index,server,browser}.ts pageHandler: dynamically imports the server bundle, calls renderToHTML(props), wraps in <head> + <body> shell with __INITIAL_PROPS__. Auto-injects request.url pathname into props as `url` (matches the React/Svelte/Vue convention from the SPA work). Polyfills globalThis.Element/Node before the bundle runs to dodge the renderer's `into instanceof Element` check (EMBER_BANDAID #3, upstream PR emberjs/ember.js#21364 to remove the dependency). index/server/browser are thin re-exports. What Phase 1 does NOT ship: streaming, slots, islands, HMR-cache-dirty handling, convention error rendering, build.ts integration. All deferred to Phase 1.5/2/3 per EMBER_PLAN.md §2.6.
EMBER_PLAN.md - §0.1 stage-3 decorator finding: documented during the Phase 1 dryrun. Glimmer's @Tracked / @service / @action surface still expects legacy/TypeScript decorator semantics. Bun.Transpiler defaults to TC39 stage-3, so compileEmber.ts sets experimentalDecorators: true. Notes what to watch for upstream and the contingency if Ember never migrates (we ship our own shim). - §0.2 RFC #1178 watch: new section. Surfaced during the upstream review on emberjs/ember.js#21364. The RFC deprecates env.isInteractive, env.hasDOM, and non-Document `document` values — all three of which our Phase 1 adapter currently uses. Documents the migration target (happy-dom matching vite-ember-ssr) for a Phase 1.5 / 1.6 follow-up. Both EMBER_BANDAID #3 (Element polyfill) and #4 (simple-dom serializer split) dissolve in that single migration. - §2.1–2.6 Phase 1 status: marked the items that shipped in Phase 1, noted what's deferred to Phase 1.5 vs Phase 2/3 with the actual reasons (most are scope-driven: streaming, islands, HMR all wait for their own phase). EMBER_BANDAID.md (new) - #1 Bun @-prefix resolver: workaround in buildEmberVendor.ts/compileEmber.ts; tracks oven-sh/bun#30187 for the unblock. - #2 @embroider/macros virtualization: not a true bandaid (correct pattern for non-babel pipelines); kept for posterity. - #3 globalThis.Element polyfill: workaround in pageHandler.ts and the server harness; tracks emberjs/ember.js#21363 + emberjs/ember.js#21364 (the issue + PR we filed). - #4 @simple-dom/serializer peer dep: not a bandaid (correct split); kept for the surprise it caused during dryrun. - Maintenance protocol: when an upstream fix lands, run the dryrun without the bandaid in place; if green, follow the entry's unblock steps and delete it. Cross-referenced via "EMBER_BANDAID #N" comments at every call site (grep -r EMBER_BANDAID src/ returns 4 hits).
Fixes #21363.
renderComponentdoesinto instanceof Elementin two places to discriminateCursorfromElement/SimpleElementin theIntoTargetunion. That works in a browser (whereElementis a built-in) and inside FastBoot (which patches a simulatedElementontoglobalThis), but throwsReferenceError: Element is not definedon a plain Node or Bun host that callsrenderComponentwith a fresh@simple-dom/documentnode asinto.The function already has an
intoTarget()helper a few lines up that classifies the same union without touching any global:This PR aligns the two
into instanceof Elementoutliers with that helper.Changes
innerHTMLclearing becomes a structural check —!('element' in into) && 'innerHTML' in into— so it fires only on a real DOMElementand skips onSimpleElement(noinnerHTMLsetter) andCursor(has.element).parentElementextraction callsintoTarget(into)directly instead of duplicating the discriminator. Drive-by fix: the previous(into as Cursor).elementcast on the else branch was incorrect forSimpleElementinput —SimpleElementhas no.elementproperty — though that path is rare in practice.Behavior
intoshapeElement(browser)innerHTML, uses self asparentElementSimpleElement(server, no FastBoot)Element is not definedinnerHTMLclearing, uses self asparentElementCursorinnerHTMLclearing, uses.elementElement(browser, noglobalThis.Elementfor some reason)Element is not definedNo behavior change for in-browser callers. New behavior for non-browser hosts that pass
SimpleElementdirectly.Test
Adds a regression test in
render-component-test.tsthat:globalThis.Elementand sets it toundefined.renderComponentwith a real DOM target (which would have crashed atinto instanceof undefined).globalThis.Element.Notes
renderComponentfrom bare Node / Bun / edge / worker hosts that supply a simple-dom Document viaenv.document.Happy to iterate on review feedback.