fix: constructed-stylesheet shadow runs and missing type declarations - #459
Open
jgerigmeyer wants to merge 8 commits into
Open
fix: constructed-stylesheet shadow runs and missing type declarations#459jgerigmeyer wants to merge 8 commits into
jgerigmeyer wants to merge 8 commits into
Conversation
`patchAndPolyfillConstructedStylesheets()` set up its own `polyfill()` runs with an explicit options object, so `polyfill()` never fell back to `window.ANCHOR_POSITIONING_POLYFILL_OPTIONS`. Global options such as `positionAreaContainingBlock: false` were silently ignored for shadow roots with adopted stylesheets. Follow-up to #443, which fixed the symptom but left three problems behind: - The re-patch guard compared `descriptor.set` against a value read from that same descriptor, so it was always true and every call stacked another wrapper on the `adoptedStyleSheets` setter. Combined with the `patchedHosts` short-circuit, a second call's options were silently discarded. Track the patched state in a module flag instead. - Options were captured when the patch was installed, so a global set afterwards was ignored — despite the docs telling callers to install the patch as early as possible. Read them at run time instead. - A global `elements` list was forwarded to the shadow runs, where it makes `fetchCSS` skip adopted stylesheets entirely, disabling the very feature this function provides. Override it alongside `roots`, and omit both from the accepted options type so passing them is a compile error rather than a silent no-op. Adds unit coverage for the options contract, and extends the e2e tests to assert the target is actually positioned rather than just unwrapped.
`patchHostConnectedCallback()` wrapped the host element's own `connectedCallback` property, intending to position the shadow root once the host was connected and its shadow DOM populated. That wrapper never ran: custom element lifecycle callbacks are looked up when the element is defined and stored on the definition, so assigning `host.connectedCallback` afterwards has no effect on the reaction. Everything kept working only because of the `host.isConnected` branch, which covers the common case of adopting from within `connectedCallback`. A host that adopts while disconnected — the familiar pattern of building the shadow root in the constructor — was never polyfilled at all. Watch for the host entering the document with a shared MutationObserver instead, which disconnects itself once no hosts are pending. Observer records are delivered after the insertion's `connectedCallback` reactions have run, so the shadow DOM is populated by then. The e2e test asserts the generated `<polyfill-position-area>` wrapper rather than geometry. An unresolved `position-area` leaves the target at its static position, which for a target that directly follows its anchor in flow is where the anchored position would put it anyway — geometry assertions there pass with or without a polyfill run.
Two packaging bugs left `dist/` in a state that didn't match what `package.json` promises. `src/@types/global.d.ts` declared the `Window` properties the README tells consumers to set, but `tsc` does not copy input `.d.ts` files to `outDir`, and nothing in the emitted output referenced it. Consumers following the documented configuration example got `TS2339: Property 'ANCHOR_POSITIONING_POLYFILL_OPTIONS' does not exist on type 'Window'`. Move it to a compiled module and pull it into each entry with a side-effect import, which `tsc` preserves in the declaration output, so `dist/index.d.ts` and `dist/index-fn.d.ts` now reference `./global.js`. Only the library builds set `emptyOutDir: false`, so `build:demo` — the first step of `build` — wiped any declarations a previous `npm run types` had emitted. `prepack` was correct only by ordering luck. Make every build additive and empty `dist/` once, up front, via a `clean` step. Verified against a consumer installed from `npm pack`: it typechecks the documented `window.ANCHOR_POSITIONING_POLYFILL_OPTIONS` assignment and rejects an unknown option, and fails as before when the side-effect import is removed.
✅ Deploy Preview for anchor-polyfill ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for anchor-position-wpt canceled.
|
The MutationObserver watching `document` for pending hosts missed the common case it was meant to cover: mutation records don't cross shadow boundaries, so a host appended into another component's shadow root produced no record and was never positioned. Nesting custom elements is the normal composition pattern, so that was most of the case. Patch `customElements.define` instead, wrapping `connectedCallback` on the constructor's prototype before the registry captures it. That is the hook the original code reached for and couldn't get after the fact, and it fires wherever the host is connected. It also removes the observer's unbounded `pendingHosts` map, which pinned every host that adopted a stylesheet and was then discarded without connecting, and kept a document-wide subtree observer alive for the lifetime of the page. The trade-off is that a host adopting while disconnected is now only positioned if it is a custom element defined after this runs. The documented requirement was already to call this before defining elements; the README now says why, and notes the non-custom-element case. The nested case is covered by an e2e test only: jsdom delivers mutation records across shadow boundaries, so the equivalent unit test passes with or without the fix.
jgerigmeyer
marked this pull request as ready for review
August 12, 2026 21:04
Contributor
|
@jpzwarte Can you take a look at this PR? Thanks! |
jamesnw
reviewed
Aug 14, 2026
jpzwarte
reviewed
Aug 15, 2026
| patchedHosts.add(host); | ||
| customElementsPatched = true; | ||
|
|
||
| const originalDefine = customElements.define; |
Contributor
There was a problem hiding this comment.
This only works for the global custom elements registry, not any scoped custom element registries. Let's look at that in a new PR.
Contributor
There was a problem hiding this comment.
Can we patch CustomElementRegistry.prototype.define instead perhaps? That would then work for scoped custom element registries as well?
Member
Author
Contributor
There was a problem hiding this comment.
I can't contribute directly to your branch; i have to still use a fork etc.
On my fork i've added an example to shadow-dom.html:
Contributor
There was a problem hiding this comment.
I've created a PR for adding this example to your branch, but i see your branch isn't up-to-date with main, so the PR includes a bunch of unwanted changes. Perhaps merge main into this one first?
Contributor
There was a problem hiding this comment.
To be clear:
- First screenshot is Chrome 153: supports anchor positioning and scoped registries
- Second screenshot is Firefox dev edition 154: supports anchor positioning but not scoped registries
But on main the polyfill doesn't try and patch CustomElementRegistry.prototype.define, so you shouldn't se any difference there.
On this branch, you should see a difference if you switch between patching CustomElementRegistry.prototype.define and window.customElements.define.
Contributor
There was a problem hiding this comment.
Now i am confused: i don't think this is testable. There aren't any browsers that don't support CSS Anchor Positioning yet, but do support Scoped Custom Element Registries. I don't think that's a real world usecase:
https://caniuse.com/css-anchor-positioning vs https://caniuse.com/wf-scoped-custom-element-registries
It's still probably better to patch CustomElementRegistry.prototype.define, but testing the polyfill in that combination is not possible?!
Assigning `customElements.define` installs an own property that shadows `CustomElementRegistry.prototype.define`, so scoped registries created with `new CustomElementRegistry()` never got the `connectedCallback` wrapper. Patch the prototype instead, which every registry inherits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jpzwarte
approved these changes
Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes to the constructed-stylesheets shadow-DOM path and to what ships in
dist/.1. Apply polyfill options to constructed-stylesheet shadow runs (follow-up to #443)
PR #443 fixed the symptom of #442 but left three defects behind:
descriptor.setagainst a value read from that same descriptor, so it was always true. Every call topatchAndPolyfillConstructedStylesheets()stacked another wrapper on theadoptedStyleSheetssetter, and combined with thepatchedHostsshort-circuit, a second call's options were silently discarded.elementslist was forwarded into the shadow runs, where it makesfetchCSSskip adopted stylesheets entirely, disabling the feature this function exists to provide.Options are now read at run time;
elementsandrootsare overridden and omitted from the accepted options type (ConstructedStylesheetsPolyfillOptions), so passing either is a compile error rather than a silent no-op.2. Position hosts that adopt a stylesheet before being connected
patchHostConnectedCallback()wrapped the host element's ownconnectedCallbackproperty. That never ran: lifecycle callbacks are looked up when the element is defined and stored on the definition, so assigninghost.connectedCallbackafterwards has no effect on the reaction. Everything kept working only via thehost.isConnectedbranch, meaning a host that adopts while disconnected — the familiar pattern of building the shadow root in the constructor — was never polyfilled at all.customElements.defineis now patched to wrapconnectedCallbackon the constructor's prototype before the registry captures it. That fires wherever the host is connected, including inside another component's shadow root, which a document-levelMutationObservercannot see — mutation records don't cross shadow boundaries.The trade-off, now documented in the README: a host adopting while disconnected is positioned only if it is a custom element defined after this call. The documented requirement was already to call this before defining elements; a non-custom-element host with a detached shadow root is not covered, since nothing signals that it has been connected.
3. Ship type declarations and stop the demo build deleting them — Fixes #449
src/@types/global.d.tswas an input.d.ts, whichtscdoes not copy tooutDir, so consumers following the documented config example gotTS2339onwindow.ANCHOR_POSITIONING_POLYFILL_OPTIONS. Moved to a compiled module pulled in by each entry with a side-effect import, whichtscpreserves in declaration output.Separately, only the library builds set
emptyOutDir: false, sobuild:demo— the first step ofbuild— wiped any declarationsnpm run typeshad emitted.prepackwas correct only by ordering luck. Builds are now additive, with acleanstep up front.Related
window. ANCHOR_POSITIONING_POLYFILL_OPTIONSis not typed #449window.ANCHOR_POSITIONING_POLYFILL_OPTIONSis ignored whenpolyfill()is called frompatchHostConnectedCallback#442build:democould still delete the declarations fix: missing .d.ts files in dist #441 arranged to emit