mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-10-30 09:16:55 +00:00
fix(lint): error on digit-leading ids only when a selector targets them (#3980)
* fix(lint): block on digit-leading ids instead of only warning
A digit-leading id (e.g. id="1-hook") breaks any bare `#1-hook` CSS
selector with a SyntaxError, including inline GSAP string selectors
like tl.fromTo("#1-hook", ...). That throw silently aborts the rest of
the <script> block that built the selector, so the referenced element
never animates — but the render itself still reports success, and the
freeze has no visible error anywhere.
id_requires_css_escape already detects this correctly; it was only
"warning" severity, which hyperframes render and hyperframes check
both treat as non-blocking by default. Promote it to "error" so
`hyperframes check` (which always treats errors as blocking) catches
this unconditionally instead of only under --strict.
Co-Authored-By: Miguel Ángel <miguel.sierra@heygen.com>
* refactor(lint): drop the digit-leading id severity comments
* fix(lint): error on digit-leading ids only when a selector targets them
* fix(lint): say a CSS-only digit selector drops the rule instead of throwing
---------
Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
This commit is contained in:
co-authored by
Miguel Ángel
parent
1248639e71
commit
385e9cf337
@@ -78,19 +78,41 @@ describe("core rules", () => {
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("warns when an id starts with a digit and is unsafe in a hash selector", async () => {
|
||||
it.each([
|
||||
["no selector uses it", "", "", "warning"],
|
||||
["a CSS rule targets it", "#123-frame { opacity: 0; }", "", "error"],
|
||||
["a GSAP string targets it", "", 'gsap.to("#123-frame", { x: 1 });', "error"],
|
||||
["querySelector targets it", "", 'document.querySelector(".a #123-frame");', "error"],
|
||||
["only url(#id) references it", ".a { mask: url(#123-frame); }", "", "warning"],
|
||||
["only a longer id is selected", "#123-frame-2 { opacity: 0; }", "", "warning"],
|
||||
["getElementById looks it up", "", 'document.getElementById("123-frame");', "warning"],
|
||||
["the CSS selector is escaped", "#\\31 23-frame { opacity: 0; }", "", "warning"],
|
||||
[
|
||||
"the script selector is escaped",
|
||||
"",
|
||||
'document.querySelector("#\\\\31 23-frame");',
|
||||
"warning",
|
||||
],
|
||||
[
|
||||
"CSS.escape builds the selector",
|
||||
"",
|
||||
'document.querySelector(`#${CSS.escape("123-frame")}`);',
|
||||
"warning",
|
||||
],
|
||||
])("rates a digit-leading id by selector use: %s", async (_case, css, js, severity) => {
|
||||
const html = `
|
||||
<html><body>
|
||||
<div data-composition-id="c1" data-width="1920" data-height="1080">
|
||||
<style>${css}</style>
|
||||
<div id="123-frame"></div>
|
||||
</div>
|
||||
<script>window.__timelines = {};</script>
|
||||
<script>window.__timelines = {}; ${js}</script>
|
||||
</body></html>`;
|
||||
|
||||
const result = await lintHyperframeHtml(html);
|
||||
const finding = result.findings.find((item) => item.code === "id_requires_css_escape");
|
||||
|
||||
expect(finding?.severity).toBe("warning");
|
||||
expect(finding?.severity).toBe(severity);
|
||||
expect(finding?.elementId).toBe("123-frame");
|
||||
expect(finding?.fixHint).toContain("CSS.escape");
|
||||
});
|
||||
|
||||
@@ -83,6 +83,44 @@ function repeatedDescendantId(selector: string): string | null {
|
||||
return repeated;
|
||||
}
|
||||
|
||||
// Unescaped `#<digit...>` id selectors: what querySelector and GSAP reject. `#\31 -x` is valid and skipped.
|
||||
function addUnescapedDigitIds(selector: string, ids: Set<string>): void {
|
||||
try {
|
||||
selectorParser((root) => {
|
||||
root.walkIds((node) => {
|
||||
if (/^#\d/.test(node.toString().trim())) ids.add(node.value);
|
||||
});
|
||||
}).processSync(selector);
|
||||
} catch {
|
||||
// An unparseable selector targets nothing we can name.
|
||||
}
|
||||
}
|
||||
|
||||
const SELECTOR_CALL_PATTERN =
|
||||
/\.(?:querySelector(?:All)?|closest|matches|to|from|fromTo|set)\s*\(\s*(["'`])((?:\\.|(?!\1)[^\\])*)\1/g;
|
||||
|
||||
function digitIdsTargetedBySelectors(
|
||||
styles: LintContext["styles"],
|
||||
scripts: LintContext["scripts"],
|
||||
): Set<string> {
|
||||
const ids = new Set<string>();
|
||||
for (const style of styles) {
|
||||
try {
|
||||
postcss.parse(style.content).walkRules((rule) => {
|
||||
for (const selector of rule.selectors) addUnescapedDigitIds(selector, ids);
|
||||
});
|
||||
} catch {
|
||||
// css_parse_error reports this block.
|
||||
}
|
||||
}
|
||||
for (const script of scripts) {
|
||||
for (const match of stripJsComments(script.content).matchAll(SELECTOR_CALL_PATTERN)) {
|
||||
addUnescapedDigitIds(match[2] ?? "", ids);
|
||||
}
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
function resolvedRuleSelectors(rule: postcss.Rule): string[] {
|
||||
let ancestor: postcss.AnyNode | undefined = rule.parent;
|
||||
while (ancestor && ancestor.type !== "rule") ancestor = ancestor.parent;
|
||||
@@ -382,15 +420,20 @@ function describeRootDimensionsDrift(
|
||||
|
||||
export const coreRules: Array<(ctx: LintContext) => HyperframeLintFinding[]> = [
|
||||
// id_requires_css_escape
|
||||
({ tags }) => {
|
||||
({ tags, styles, scripts }) => {
|
||||
const findings: HyperframeLintFinding[] = [];
|
||||
let targeted: Set<string> | undefined;
|
||||
for (const tag of tags) {
|
||||
const id = readAttr(tag.raw, "id");
|
||||
if (!id || !/^\d/.test(id)) continue;
|
||||
targeted ??= digitIdsTargetedBySelectors(styles, scripts);
|
||||
const used = targeted.has(id);
|
||||
findings.push({
|
||||
code: "id_requires_css_escape",
|
||||
severity: "warning",
|
||||
message: `id="${id}" starts with a digit, so the common selector \`#${id}\` throws a SyntaxError in querySelector().`,
|
||||
severity: used ? "error" : "warning",
|
||||
message: used
|
||||
? `id="${id}" starts with a digit, and the selector \`#${id}\` used in this composition is invalid: querySelector and GSAP throw a SyntaxError, and CSS drops the rule.`
|
||||
: `id="${id}" starts with a digit, so the common selector \`#${id}\` throws a SyntaxError in querySelector().`,
|
||||
elementId: id,
|
||||
fixHint:
|
||||
"Rename the id to start with a letter (recommended), or build selectors with `#${CSS.escape(id)}` at runtime.",
|
||||
|
||||
Reference in New Issue
Block a user