Skip to content

[BUGFIX] DocumentFragment support - #21551

Open
NullVoxPopuli wants to merge 5 commits into
mainfrom
nvp/in-element-document-fragment
Open

NullVoxPopuli wants to merge 5 commits into
mainfrom
nvp/in-element-document-fragment

Conversation

@NullVoxPopuli

@NullVoxPopuli NullVoxPopuli commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This is a feature gap, because we already support {{domNode}} that someone used createElement with, and the limitations of that should be the same here with a fragment

Continuation of:

Which is a continuation of:

This is required to unblock using shadow-dom with out whack hacks.

See:

Demo of this PR working (REPL)

test code for the repl
import Component from '@glimmer/component';
import { trackedObject } from '@ember/reactive/collections';

const state = trackedObject({
  count: 0,
  inc: () => state.count++,
});


const fragment = document.createDocumentFragment();

setInterval(() => {
    console.log(fragment.textContent)
}, 250);

<template>
  check the console 

  <hr>
  as a separate PR we need to make fragments renderable
  today, folks would need a modifier to append the fragment to the DOM, 
  which could cause Glimmer to get confused by someone else managing DOM:
  {{fragment}}

  {{#in-element fragment}}
    text in a fragment
    {{log "in-element body"}}
    {{state.count}}

    <button onclick={{state.inc}}>inc</button>
  {{/in-element}}

  <hr> 
  
  outside the fragement is useful for testing reactivity
  <button onclick={{state.inc}}>inc</button>
</template> 
REPL deploy info

CI run: https://github.com/NullVoxPopuli/limber/actions/runs/31221384086
Preview PR: NullVoxPopuli/limber#2211
REPL with this change: https://test-ember-source-nvp-in-ele.limber-glimdown.pages.dev/

Perf is good.

image

Out of scope

  • insertBefore values other than null/undefined (unchanged from Element destinations).

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

📊 Size report

Tarball size1.2 MB1.2 MB

dist/dev   0.5%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/api-{hash}.js 10 kB / 2.1 kB 209%↑31 kB / 238%↑7.2 kB
./packages/shared-chunks/index-{hash}.js 51.9 kB / 13 kB 20%↑62.6 kB / -3.32%↓12.5 kB
./packages/shared-chunks/normalize-{hash}.js 2.2 kB / 617 B 30%↑2.9 kB / 38%↑849 B
./packages/shared-chunks/rehydrate-{hash}.js 15.6 kB / 3.1 kB 25%↑19.5 kB / 27%↑4 kB
./packages/shared-chunks/render-{hash}.js 56.1 kB / 12.2 kB 0.4%↑56.3 kB / 0.7%↑12.3 kB
./packages/shared-chunks/serialize-{hash}.js 3.6 kB / 1 kB 19%↑4.3 kB / 17%↑1.2 kB
Total (Includes all files) 2.1 MB / 494 kB 0.5%↑2.1 MB / 0.5%↑496.7 kB

dist/prod   0.5%↑

File Before (Size / Brotli) After (Size / Brotli)
./packages/shared-chunks/api-{hash}.js 25.8 kB / 5.8 kB 18%↑30.5 kB / 23%↑7.1 kB
./packages/shared-chunks/normalize-{hash}.js 2.2 kB / 617 B 30%↑2.9 kB / 38%↑849 B
./packages/shared-chunks/rehydrate-{hash}.js 15.6 kB / 3.1 kB 25%↑19.5 kB / 27%↑4 kB
./packages/shared-chunks/render-{hash}.js 52.4 kB / 11.3 kB 0.4%↑52.6 kB / 0.7%↑11.4 kB
./packages/shared-chunks/serialize-{hash}.js 3.6 kB / 1 kB 19%↑4.3 kB / 16%↑1.2 kB
Total (Includes all files) 1.9 MB / 451.2 kB 0.5%↑1.9 MB / 0.6%↑453.9 kB

smoke-tests/v2-app-template/dist   0.8%↑

File Before (Size / Brotli) After (Size / Brotli)
./assets/api-{hash}.js 291.4 kB / 78.2 kB 0.9%↑294.1 kB / 0.7%↑78.8 kB
Total (Includes all files) 344.2 kB / 95.4 kB 0.8%↑346.9 kB / 0.6%↑96 kB

smoke-tests/v2-app-hello-world-template/dist   0.7%↑

File Before (Size / Brotli) After (Size / Brotli)
./assets/main-{hash}.js 134.9 kB / 37.6 kB 0.7%↑135.9 kB / 0.7%↑37.9 kB
Total (Includes all files) 135.3 kB / 37.8 kB 0.7%↑136.2 kB / 0.7%↑38.1 kB

🤖 This report was automatically generated by wyvox/pkg-size

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

someone's editor doesn't strip unneeded invisible characters :P


`{{in-element}}` requires a single positional argument:

- `destinationElement` -- the DOM element to render into. It must exist at the time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meaningful change here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"a"

@NullVoxPopuli

This comment was marked as outdated.

Comment thread packages/@glimmer/node/lib/serialize-builder.ts
NullVoxPopuli-ai-agent pushed a commit to NullVoxPopuli-ai-agent/ember.js that referenced this pull request Aug 19, 2026
Continuation of emberjs#21551.

`{{fragment}}` inserted the fragment's children and returned bounds around
them. Inserting a DocumentFragment empties it, so the fragment was no longer
an address that later renders could target: `{{#in-element fragment}}` after
`{{fragment}}` rendered into a detached fragment and nothing appeared.

Rendering a fragment now brackets its children with a pair of comment markers
and records that pair as the fragment's region. `{{#in-element fragment}}`
renders into the region instead of the fragment, so content lands where the
fragment was rendered, stays reactive, and stays between its own markers while
sibling regions keep their place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@NullVoxPopuli
NullVoxPopuli force-pushed the nvp/in-element-document-fragment branch from a515761 to e449a97 Compare August 19, 2026 19:09
@NullVoxPopuli NullVoxPopuli changed the title [BUGFIX] {{#in-element}} with DocumentFragment targets [BUGFIX] DocumentFragment support - pairity with Element placed via {{ }} and via {{#in-element}} Aug 19, 2026
@NullVoxPopuli

Copy link
Copy Markdown
Contributor Author

BobrImperator added a commit to mainmatter/ember.js that referenced this pull request Sep 2, 2026
{
input: (test, isHTML) => {
if (isHTML) {
return test.makeFragment([test.makeElement('p', 'one'), test.makeElement('p', 'two')]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda funny we've been testing with fragments all this time, but had no way to actually see them

@NullVoxPopuli
NullVoxPopuli marked this pull request as ready for review September 3, 2026 13:21
@NullVoxPopuli NullVoxPopuli changed the title [BUGFIX] DocumentFragment support - pairity with Element placed via {{ }} and via {{#in-element}} [BUGFIX] DocumentFragment support Sep 3, 2026
@NullVoxPopuli
NullVoxPopuli removed this pull request from stack #21593 September 9, 2026 22:31
…nt-what-happens-if-someone-mutates-fragement-while-in-use

DocumentFragment tests when fragment is mutated manually during render
…nt-chaos-tests

DocumentFragment Chaos Tests

This branch has not been deployed

No deployments
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.

2 participants