Skip to content

fix(dom): stop stamping live stylesheet <link>s, duplicating them (PER-10610) #2406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pranavz28 merged 1 commit into from
Aug 31, 2026
Merged

fix(dom): stop stamping live stylesheet <link>s, duplicating them (PER-10610) #2406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/dom/src/clone-dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function cloneElementWithoutLifecycle(element) {
}

export function cloneNodeAndShadow(ctx) {
let { dom, disableShadowDOM, forceShadowAsLightDOM, resources, cache, enableJavaScript } = ctx;
let { dom, disableShadowDOM, forceShadowAsLightDOM, resources, cache, enableJavaScript, styleSheetClones } = ctx;
// clones shadow DOM and light DOM for a given node
let cloneNode = (node, parent) => {
try {
Expand All @@ -73,6 +73,10 @@ export function cloneNodeAndShadow(ctx) {

let clone = cloneElementWithoutLifecycle(node);

if (styleSheetClones && (node.nodeName === 'LINK' || node.nodeName === 'STYLE')) {
styleSheetClones.set(node, clone);
}

// Custom-element :state() is captured by the fallback path in
// serialize-custom-states.js (live el.matches against state names
// discovered in CSS) — no clone-time fast path remains.
Expand Down
7 changes: 0 additions & 7 deletions packages/dom/src/prepare-dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,8 @@ export function markElement(domElement, disableShadowDOM, forceShadowAsLightDOM)
// Custom elements with ElementInternals or closed shadow roots also get
// stamped so the post-clone state-fallback can locate their clones.
let tagName = domElement.tagName?.toLowerCase();
// Stylesheet <link>s are stamped so the pseudo-class serializer can locate
// the sheet's clone element and anchor its rewritten interactive-state rules
// immediately after it — preserving the sheet's original cascade position
// instead of appending at the end of <head> (PER-10077).
let isStylesheetLink = tagName === 'link' &&
/(^|\s)stylesheet(\s|$)/i.test(domElement.getAttribute('rel') || '');
if (
['input', 'textarea', 'select', 'iframe', 'canvas', 'video', 'style', 'dialog'].includes(tagName) ||
isStylesheetLink ||
isCustomElement(domElement)
) {
if (!domElement.getAttribute('data-percy-element-id')) {
Expand Down
3 changes: 2 additions & 1 deletion packages/dom/src/serialize-cssom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function createStyleResource(styleSheet) {
}

export function serializeCSSOM(ctx) {
let { dom, clone, resources, cache, warnings, ignoreStyleSheetSerializationErrors } = ctx;
let { dom, clone, resources, cache, warnings, ignoreStyleSheetSerializationErrors, styleSheetClones } = ctx;
// in-memory CSSOM into their respective DOM nodes.
let styleSheets = null;
// catch error in case styleSheets property is not available (overwritten to throw error)
Expand Down Expand Up @@ -81,6 +81,7 @@ export function serializeCSSOM(ctx) {

cloneOwnerNode.parentNode.insertBefore(style, cloneOwnerNode.nextSibling);
cloneOwnerNode.remove();
styleSheetClones?.set(styleSheet.ownerNode, style);
} catch (err) {
handleErrors(err, 'Error serializing stylesheet: ', cloneOwnerNode, {
styleId: styleId
Expand Down
1 change: 1 addition & 0 deletions packages/dom/src/serialize-dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ export function serializeDOM(options) {
warnings: new Set(),
hints: new Set(),
cache: new Map(),
styleSheetClones: new WeakMap(),
shadowRootElements: [],
enableJavaScript,
disableShadowDOM,
Expand Down
21 changes: 12 additions & 9 deletions packages/dom/src/serialize-pseudo-classes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,11 +510,11 @@ function extractPseudoClassRules(ctx) {
styleElement.textContent = rewrittenRules.join('\n');

if (owner === null) {
injectAtSheetPosition(ctx.clone, sheet, styleElement, ctx.clone.head || ctx.clone.querySelector('head'));
injectAtSheetPosition(ctx, ctx.clone, sheet, styleElement, ctx.clone.head || ctx.clone.querySelector('head'));
} else {
const cloneHost = cloneByPercyId.get(owner.getAttribute('data-percy-element-id'));
if (cloneHost && cloneHost.shadowRoot) {
injectAtSheetPosition(cloneHost.shadowRoot, sheet, styleElement, cloneHost.shadowRoot);
injectAtSheetPosition(ctx, cloneHost.shadowRoot, sheet, styleElement, cloneHost.shadowRoot);
}
}
}
Expand All @@ -523,13 +523,16 @@ function extractPseudoClassRules(ctx) {
// Insert an interactive-states <style> immediately AFTER its source sheet's
// clone element so the copy keeps the sheet's original source-order rank —
// later stylesheets still win equal-specificity ties, exactly as on the live
// page (PER-10077). The sheet's ownerNode (<style> / <link rel=stylesheet>) is
// stamped with a data-percy-element-id, which serialize-cssom preserves on any
// node it rebuilds in place. Falls back to appending at the scope's end when
// the sheet has no locatable clone anchor.
function injectAtSheetPosition(scopeRoot, sheet, styleElement, fallbackTarget) {
const anchorId = sheet.ownerNode.getAttribute('data-percy-element-id');
const anchor = anchorId ? scopeRoot.querySelector(`[data-percy-element-id="${anchorId}"]`) : null;
// page (PER-10077).
function injectAtSheetPosition(ctx, scopeRoot, sheet, styleElement, fallbackTarget) {
const ownerNode = sheet.ownerNode;
let anchor = ownerNode ? ctx.styleSheetClones?.get(ownerNode) : null;

if (!anchor?.parentNode && ownerNode) {

@pranavz28 pranavz28 Aug 31, 2026

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.

[Low] data-percy-element-id fallback anchor is now unreachable for <link> owners

Since prepare-dom.js no longer stamps data-percy-element-id on stylesheet <link>s, this middle fallback step (scopeRoot.querySelector('[data-percy-element-id=...]')) can never resolve an anchor for a <link>-owned sheet anymore — only for <style> owners, which are still stamped. In production this is harmless: cloneNodeAndShadow always populates ctx.styleSheetClones before this runs, so the WeakMap lookup succeeds and this branch is not reached for links. It is exercised today only by a pre-existing hand-built-ctx unit test (cascade position of injected rules) that pre-stamps a <style> tag directly, so nothing currently regresses. Not a bug — the PR description documents this as an intentional 3-step degrade chain (map → id lookup → end-of-head) — flagging only because the middle step is dead for one of the two owner-node kinds it nominally handles.

Suggestion: No action required; optional follow-up would be a short inline note (or drop the redundant middle step for <link>-only lookups) so a future reader doesn't assume this fallback still helps stylesheet links. Given the repo convention here of no explanatory code comments, this is best left as-is or addressed only if it becomes confusing in practice.

Reviewer: code-review (built-in fallback)

@pranavz28 pranavz28 Aug 31, 2026

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.

Adjudicated: accurate observation, keeping the code as-is. Recording the reasoning rather than implementing either suggestion.

The observation is correct — for a <link>-owned sheet this middle step can no longer resolve, because not stamping links is the whole point of the fix. But the suggestion to "drop the redundant middle step" would be a regression, because the step is not redundant: it is load-bearing for the other owner-node kind and for direct callers.

ctx.styleSheetClones?.get(...) is optional-chained precisely because serializePseudoClasses is a public entry point that callers invoke with their own ctx. In test/serialize-pseudo-classes.test.js there are 33 calls to serializePseudoClasses(...) and zero that supply styleSheetClones. For every one of those, the WeakMap lookup yields undefined and the data-percy-element-id step is the only thing that resolves a <style> anchor — including the cascade position of injected rules spec that guards PER-10077. Dropping it would break them and silently downgrade those callers to append-at-end-of-head.

So the accurate framing is not "dead middle step" but "step 1 covers <link> and <style> for the serializeDOM() path; step 2 covers <style> for callers that build their own ctx." Each step degrades to something correct instead of throwing, which is the documented intent.

On the optional inline note: declining per this repo's convention that rationale lives in the PR description, not the diff. The fallback chain and its ordering are already written up there under "Fallback chain, in order."

No code change.

const anchorId = ownerNode.getAttribute('data-percy-element-id');
anchor = anchorId ? scopeRoot.querySelector(`[data-percy-element-id="${anchorId}"]`) : null;
}

const anchorParent = anchor ? anchor.parentNode : null;
if (anchorParent) {
anchorParent.insertBefore(styleElement, anchor.nextSibling);
Expand Down
1 change: 1 addition & 0 deletions packages/dom/test/assets/hover-anchor.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.lbtn:hover { color: red }
57 changes: 51 additions & 6 deletions packages/dom/test/serialize-dom.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,22 +883,67 @@ describe('serializeDOM', () => {
});
});

describe('stylesheet <link> stamping (PER-10077 cascade anchoring)', () => {
it('stamps only stylesheet <link>s with a percy element id', () => {
// The pseudo-class serializer anchors its rewritten rules after a sheet's
// clone element, found via data-percy-element-id — so stylesheet links
// need one. Non-stylesheet links (preload, icon, …) do not.
describe('stylesheet <link> handling (PER-10610)', () => {
it('leaves a live stylesheet <link> equal to what a head manager rendered', () => {
withExample('<link rel="stylesheet" href="data:text/css,.lx{color:red}">', { withShadow: false });
let link = document.querySelector('link[rel="stylesheet"][href^="data:text/css,.lx"]');
let rendered = link.cloneNode();

serializeDOM();

expect(link.isEqualNode(rendered)).toBe(true);
});

it('does not stamp stylesheet <link>s in the serialized output', () => {
withExample(
'<link rel="stylesheet" href="data:text/css,.lx{color:red}">' +
'<link rel="preload" as="style" href="data:text/css,.ly{color:red}">' +
'<link href="data:text/css,.lz{color:red}">',
{ withShadow: false }
);
let $ = parseDOM(serializeDOM().html, 'plain');
expect($('link[rel="stylesheet"]')[0].getAttribute('data-percy-element-id')).toBeTruthy();
expect($('link[rel="stylesheet"]')[0].getAttribute('data-percy-element-id')).toBeNull();
expect($('link[rel="preload"]')[0].getAttribute('data-percy-element-id')).toBeNull();
expect($('link:not([rel])')[0].getAttribute('data-percy-element-id')).toBeNull();
});

it('does not accumulate <link>s when a head manager reconciles between snapshots', () => {
withExample('<link rel="stylesheet" href="data:text/css,.lrec{color:red}">', { withShadow: false });
let link = document.querySelector('link[rel="stylesheet"][href^="data:text/css,.lrec"]');
let rendered = link.cloneNode();
let reconcile = () => {
if (!link.isEqualNode(rendered)) link.after(rendered.cloneNode());
};

serializeDOM();
reconcile();
let html = serializeDOM().html;

expect(html.split('data:text/css,.lrec{').length - 1).toEqual(1);
});

it('still anchors a rewritten copy after its source <link>, not at end of head (PER-10077)', async () => {
withExample(
'<link rel="stylesheet" href="base/test/assets/hover-anchor.css">' +
'<style>.lbtn.later { color: blue }</style>' +
'<button class="lbtn later">go</button>',
{ withShadow: false }
);
let link = document.querySelector('link[href="base/test/assets/hover-anchor.css"]');
await new Promise((resolve, reject) => {
if (link.sheet) return resolve();
link.addEventListener('load', resolve, { once: true });
link.addEventListener('error', () => reject(new Error('hover-anchor.css failed to load')), { once: true });
});

let html = serializeDOM({ enablePseudoClassSerialization: true }).html;
let sourceIdx = html.indexOf('hover-anchor.css');
let copyIdx = html.indexOf('.lbtn[data-percy-hover]');
let laterIdx = html.indexOf('.lbtn.later');
expect(copyIdx).toBeGreaterThan(-1);
expect(copyIdx).toBeGreaterThan(sourceIdx);
expect(copyIdx).toBeLessThan(laterIdx);
});
});

describe(':state() CSS rewriting', () => {
Expand Down
Loading