diff --git a/README.md b/README.md index eb606c44..bde6c2ea 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,17 @@ style properties at runtime. If you are using inline styles to set anchor-related properties and the polyfill isn't working, verify that the inline styles are actually showing up in the DOM. +### Invalid CSS + +Some types of invalid CSS will cause the polyfill to throw an error. In these +cases, the polyfill will report any parse errors encountered in the console as +warnings. This will be followed by the error thrown by the polyfill. + +The polyfill can't determine which parse error caused the polyfill error, but +please resolve any reported parse errors before opening a bug. We also recommend +using a CSS linter like [Stylelint](https://stylelint.io/) or +[@eslint/css](https://github.com/eslint/css). + ## Sponsor OddBird's OSS Work At OddBird, we love contributing to the languages & tools developers rely on. diff --git a/src/cascade.ts b/src/cascade.ts index 5130e99b..2ffeda8c 100644 --- a/src/cascade.ts +++ b/src/cascade.ts @@ -50,7 +50,7 @@ function shiftUnsupportedProperties(node: CssNode, block?: Block) { export function cascadeCSS(styleData: StyleData[]) { for (const styleObj of styleData) { let changed = false; - const ast = getAST(styleObj.css); + const ast = getAST(styleObj.css, true); walk(ast, { visit: 'Declaration', enter(node) { diff --git a/src/polyfill.ts b/src/polyfill.ts index cad04dde..7398c141 100644 --- a/src/polyfill.ts +++ b/src/polyfill.ts @@ -32,6 +32,7 @@ import { type SizingProperty, } from './syntax.js'; import { transformCSS } from './transform.js'; +import { reportParseErrorsOnFailure, resetParseErrors } from './utils.js'; const platformWithCache = { ...platform, _c: new Map() }; @@ -591,13 +592,30 @@ export async function polyfill( // fetch CSS from stylesheet and inline style let styleData = await fetchCSS(options.elements, options.excludeInlineStyles); - // pre parse CSS styles that we need to cascade - const cascadeCausedChanges = cascadeCSS(styleData); - if (cascadeCausedChanges) { - styleData = transformCSS(styleData); + let rules: AnchorPositions = {}; + let inlineStyles: Map> | undefined; + + // Reset the CSS parse errors in case the polyfill is run multiple times, and + // at the beginning in case a previous run failed. + resetParseErrors(); + + // If the polyfill fails during the steps in the try catch, it is likely due + // to invalid CSS, so report the CSS parse errors. Subsequent errors are less + // likely to be caused by parse errors. + try { + // pre parse CSS styles that we need to cascade + const cascadeCausedChanges = cascadeCSS(styleData); + if (cascadeCausedChanges) { + styleData = transformCSS(styleData); + } + // parse CSS + const parsedCSS = await parseCSS(styleData); + rules = parsedCSS.rules; + inlineStyles = parsedCSS.inlineStyles; + } catch (error) { + reportParseErrorsOnFailure(); + throw error; } - // parse CSS - const { rules, inlineStyles } = await parseCSS(styleData); if (Object.values(rules).length) { // update source code diff --git a/src/utils.ts b/src/utils.ts index 0b0660e5..58b7bb31 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -6,6 +6,7 @@ import type { List, Selector as CssTreeSelector, SelectorList, + SyntaxParseError, Value, } from 'css-tree'; import generate from 'css-tree/generator'; @@ -17,6 +18,12 @@ import type { Selector } from './dom.js'; export const INSTANCE_UUID = nanoid(); +/** Singleton to hold CSS parse errors in case polyfill fails. + * + * Not included in the store in parse.ts, as it has a different lifecycle. + */ +export const cssParseErrors = new Set() as Set; + // https://github.com/import-js/eslint-plugin-import/issues/3019 export interface DeclarationWithValue extends Declaration { @@ -26,11 +33,18 @@ export interface DeclarationWithValue extends Declaration { export function isAnchorFunction(node: CssNode | null): node is FunctionNode { return Boolean(node && node.type === 'Function' && node.name === 'anchor'); } - -export function getAST(cssText: string) { +/** + * @param cssText + * @param captureErrors `true` only on the initial parse of CSS before the + * polyfill changes it + */ +export function getAST(cssText: string, captureErrors = false) { return parse(cssText, { parseAtrulePrelude: false, parseCustomProperty: true, + onParseError: (err) => { + if (captureErrors) cssParseErrors.add(err); + }, }); } @@ -100,3 +114,24 @@ export function getSelectors(rule: SelectorList | undefined) { }) .toArray(); } + +export function reportParseErrorsOnFailure() { + if (cssParseErrors.size > 0) { + // eslint-disable-next-line no-console + console.group( + `The CSS anchor positioning polyfill was not applied due to ${ + cssParseErrors.size === 1 ? 'a CSS parse error' : 'CSS parse errors' + }.`, + ); + cssParseErrors.forEach((err) => { + // eslint-disable-next-line no-console + console.warn(err.formattedMessage); + }); + // eslint-disable-next-line no-console + console.groupEnd(); + } +} + +export function resetParseErrors() { + cssParseErrors.clear(); +} diff --git a/tests/unit/polyfill.test.ts b/tests/unit/polyfill.test.ts index 7358d22a..80741842 100644 --- a/tests/unit/polyfill.test.ts +++ b/tests/unit/polyfill.test.ts @@ -3,10 +3,12 @@ import { getAxisProperty, getPixelValue, type GetPixelValueOpts, + polyfill, resolveLogicalSideKeyword, resolveLogicalSizeKeyword, } from '../../src/polyfill.js'; import { type AnchorSide, type AnchorSize } from '../../src/syntax.js'; +import { cssParseErrors } from '../../src/utils.js'; describe('resolveLogicalSideKeyword', () => { it.each([ @@ -193,3 +195,62 @@ describe('getPixelValue [anchor-size() fn]', () => { }, ); }); + +const makeElements = (css) => { + const styleEl = document.createElement('style'); + styleEl.innerHTML = css; + return [styleEl]; +}; + +describe('polyfill', () => { + let consoleGroup; + let consoleWarn; + beforeEach(() => { + consoleGroup = vi.spyOn(console, 'group').mockImplementation(() => null); + consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => null); + }); + + afterAll(() => { + consoleGroup.mockReset(); + consoleWarn.mockReset(); + }); + it('describes parse error on polyfill failure', async () => { + const elements = makeElements('.a-[1] { position-area: end; }'); + + await expect(polyfill({ elements })).rejects.toThrow( + 'Cannot read properties', + ); + expect(cssParseErrors).toHaveLength(1); + expect(consoleGroup).toHaveBeenCalledWith( + 'The CSS anchor positioning polyfill was not applied due to a CSS parse error.', + ); + expect(consoleWarn.mock.calls[0]).toMatchInlineSnapshot(` + [ + "Parse error: Identifier is expected + 1 |.a-[1] { position-area: end; } + -----------^", + ] + `); + }); + it('does not report parse errors when polyfill succeeds', async () => { + const elements = makeElements('.a {width: var(a-[1a])}'); + + await expect(polyfill({ elements })).resolves.toBeDefined(); + expect(cssParseErrors).toHaveLength(1); + expect(consoleGroup).not.toHaveBeenCalled(); + expect(consoleWarn).not.toHaveBeenCalled(); + }); + it('parse errors do not persist across polyfill calls', async () => { + const styleEl = document.createElement('style'); + styleEl.innerHTML = '.a {width: var(a-[1a])}'; + const elements = makeElements('.a {width: var(a-[1a])}'); + + await expect(polyfill({ elements })).resolves.toBeDefined(); + expect(cssParseErrors).toHaveLength(1); + + // Call polyfill again with a parse error + const elements1 = makeElements('.a {width: var(a-[1a])}'); + await expect(polyfill({ elements: elements1 })).resolves.toBeDefined(); + expect(cssParseErrors).toHaveLength(1); + }); +}); diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index 4f8bfc5b..069419ee 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -1,6 +1,6 @@ import type * as csstree from 'css-tree'; -import { getAST, splitCommaList } from '../../src/utils.js'; +import { cssParseErrors, getAST, splitCommaList } from '../../src/utils.js'; describe('splitCommaList', () => { it('works', () => { @@ -20,3 +20,24 @@ describe('splitCommaList', () => { ]); }); }); +describe('getAST', () => { + beforeEach(() => { + cssParseErrors.clear(); + }); + it('parses valid CSS', () => { + const cssText = 'a { color: red; }'; + const ast = getAST(cssText); + expect(ast.type).toBe('StyleSheet'); + }); + + it('stores cssParseError on invalid declaration', () => { + const cssText = 'a { color; red; } '; + getAST(cssText, true); + expect(cssParseErrors.size).toBe(2); + }); + it('stores cssParseError on invalid selector', () => { + const cssText = 'a-[1] { color: red; } '; + getAST(cssText, true); + expect(cssParseErrors.size).toBe(1); + }); +});