Skip to content

Commit d6ae2c7

Browse files
crisbetoclydin
authored andcommitted
fix(@angular-devkit/build-angular): error during critical CSS inlining for external stylesheets
These changes revert the performance improvement from e88aea6 and add a test. The problem with the previous approach is that it assumes that a `link` tag processed by Critters will be preceded by a `style` tag that was inserted by Critters. This assumption might not necessarily be true if Critters is unable to resolve the content of the linked styles. Furthermore, I'm not sure if we need to optimize this code path to begin with, because it only does a shallow traversal of the `head` tag which is generally fast and is unlikely to have many direct descendants. Fixes #25419.
1 parent 449e21b commit d6ae2c7

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

Diff for: packages/angular_devkit/build_angular/src/utils/index-file/inline-critical-css.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,15 @@ class CrittersExtended extends Critters {
146146
this.conditionallyInsertCspLoadingScript(document, cspNonce);
147147
}
148148

149-
link.prev?.setAttribute('nonce', cspNonce);
149+
// Ideally we would hook in at the time Critters inserts the `style` tags, but there isn't
150+
// a way of doing that at the moment so we fall back to doing it any time a `link` tag is
151+
// inserted. We mitigate it by only iterating the direct children of the `<head>` which
152+
// should be pretty shallow.
153+
document.head.children.forEach((child) => {
154+
if (child.tagName === 'style' && !child.hasAttribute('nonce')) {
155+
child.setAttribute('nonce', cspNonce);
156+
}
157+
});
150158
}
151159

152160
return returnValue;

Diff for: packages/angular_devkit/build_angular/src/utils/index-file/inline-critical-css_spec.ts

+30
Original file line numberDiff line numberDiff line change
@@ -133,4 +133,34 @@ describe('InlineCriticalCssProcessor', () => {
133133
html { color: white; }
134134
</style>`);
135135
});
136+
137+
it('should not modify the document for external stylesheets', async () => {
138+
const inlineCssProcessor = new InlineCriticalCssProcessor({
139+
readAsset,
140+
});
141+
142+
const initialContent = `
143+
<!doctype html>
144+
<html lang="en">
145+
<head>
146+
<meta charset="utf-8">
147+
<link rel="stylesheet" href="https://google.com/styles.css" />
148+
</head>
149+
<body>
150+
<app ngCspNonce="{% nonce %}"></app>
151+
</body>
152+
</html>
153+
`;
154+
155+
const { content } = await inlineCssProcessor.process(initialContent, {
156+
outputPath: '/dist/',
157+
});
158+
159+
expect(tags.stripIndents`${content}`).toContain(tags.stripIndents`
160+
<head>
161+
<meta charset="utf-8">
162+
<link rel="stylesheet" href="https://google.com/styles.css">
163+
</head>
164+
`);
165+
});
136166
});

0 commit comments

Comments
 (0)