feat: track CSS custom properties on CSSSourceCode#411
Conversation
|
Hi @copilot!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
1 similar comment
|
Hi @copilot!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
…no-invalid-properties Implements the RFC for CSS custom property tracking on CSSSourceCode: - Add #customProperties map and #declarationVariables WeakMap - Populate during traverse() to track declarations, @Property definitions, and var() references - Add getDeclarationVariables(declaration) method - Add getClosestVariableValue(func) method - Add getVariableValues(func) method Refactor no-invalid-properties rule to use the new API: - Use getClosestVariableValue() for primary variable resolution - Pre-populate vars map from AST to support variable hoisting (fixes #199) - Fallback values take priority over other-block declarations per RFC Add comprehensive tests for all new CSSSourceCode methods. Agent-Logs-Url: https://github.com/eslint/css/sessions/e41fbbf4-0081-469f-8248-1e854213b08c Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
|
Hi @copilot!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
2 similar comments
|
Hi @copilot!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
|
Hi @copilot!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the pull request title to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by clicking 'Edit' next to the pull request title at the top of this page. Read more about contributing to ESLint here |
src/rules/no-invalid-properties.js
Outdated
| * @param {Function} visitor The visitor function to call for each node. | ||
| * @returns {void} | ||
| */ | ||
| function walkAST(node, visitor) { |
There was a problem hiding this comment.
@copilot the rule itself is doing a depth-first traversal of the AST. Doing it again is inefficient. Remove the extra traversal and use the rule methods to do any traversing that is necessary.
There was a problem hiding this comment.
Removed the walkAST function and StyleSheet() visitor. The vars map is now populated incrementally in "Rule > Block Declaration:exit" as the rule traverses. Hoisting is handled by sourceCode.getClosestVariableValue() which has access to all declarations collected during CSSSourceCode.traverse(). Commit: 63d930c
…le visitor methods instead Agent-Logs-Url: https://github.com/eslint/css/sessions/fa674c27-bfb3-46b4-8c86-407a2fef55c5 Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
nzakas
left a comment
There was a problem hiding this comment.
LGTM. Would like a second review before merging.
|
@copilot run ESLint and prettier and fix any problems so the CI passes. |
Agent-Logs-Url: https://github.com/eslint/css/sessions/067a08d1-cf1f-4d0b-ac09-9abe4691f119 Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
| /** | ||
| * Helper to create a CSSSourceCode from CSS text and trigger traversal. | ||
| * @param {string} css The CSS text. | ||
| * @returns {import("../../src/languages/css-source-code.js").CSSSourceCode} The source code instance. | ||
| */ | ||
| function createSourceCode(css) { | ||
| const sourceCode = new CSSSourceCode({ | ||
| text: css, | ||
| ast: toPlainObject(parse(css, { positions: true })), | ||
| }); | ||
|
|
||
| // trigger traversal to populate custom properties data | ||
| sourceCode.traverse(); | ||
| return sourceCode; | ||
| } |
There was a problem hiding this comment.
I think it would be clearer to move this helper to the top level, since the same test helper is repeated three times across the newly added describe test blocks.
There was a problem hiding this comment.
Moved both createSourceCode and findVarFunc to the top level, shared across all three describe blocks. Commit: 051fea2
| } | ||
|
|
||
| // Track var() references | ||
| if ( |
There was a problem hiding this comment.
Should this also handle nested var() calls in fallback positions, such as var(--a, var(--b))?
It seems that no-invalid-properties already has custom logic for resolving nested fallback var() references. If this RFC is meant to centralize that behavior in CSSSourceCode, I think this case need to be covered as well. Otherwise, rules may still need their own fallback parsing logic.
Just for reference, according to MDN, the following syntax is valid:
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/var#syntax
var(--custom-prop, var(--default-value));
var(--custom-prop, var(--default-value, red));When I tested it against a nested var() function, it failed:
it("should return nested var() function nodes", () => {
const css = "a { color: var(--my-color-1, var(--my-color-2)); }";
const sourceCode = createSourceCode(css);
const decl = sourceCode.ast.children[0].block.children[0];
const vars = sourceCode.getDeclarationVariables(decl);
console.log(decl.value.children[0]);
console.log(vars);
assert.strictEqual(vars.length, 2);
assert.strictEqual(vars[0].children[0].name, "--my-color-1");
assert.strictEqual(vars[1].children[0].name, "--my-color-2");
});The stdout is logged as follows:
{
type: 'Function',
loc: {
source: '<unknown>',
start: { offset: 11, line: 1, column: 12 },
end: { offset: 47, line: 1, column: 48 }
},
name: 'var',
children: [
{ type: 'Identifier', loc: [Object], name: '--my-color-1' },
{ type: 'Operator', loc: [Object], value: ',' },
{ type: 'Raw', loc: [Object], value: ' var(--my-color-2)' }
]
}
[
{
type: 'Function',
loc: { source: '<unknown>', start: [Object], end: [Object] },
name: 'var',
children: [ [Object], [Object], [Object] ]
}
]There was a problem hiding this comment.
Added #extractNestedVarRefs private method that re-parses Raw fallback text using the CSS parser and tracks any nested var() references found. It handles arbitrary nesting depth by recursing. Location info is adjusted to map back to the original source positions via adjustLoc/adjustAllLocs helpers. Two new tests cover single-level and 3-level deep nesting. Commit: 051fea2
|
@copilot Address @lumirlumir's feedback. |
Agent-Logs-Url: https://github.com/eslint/css/sessions/c7fb3f76-2d5b-4f30-bbcb-051fb15694b7 Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
Both items addressed in 051fea2 — test helpers moved to top level, and nested |
Implements the CSS variable tracking RFC, centralizing custom property tracking on
CSSSourceCodeso rules don't duplicate this logic. Also fixes variable hoisting —var()references before their declaration now resolve correctly.CSSSourceCode— new APIThree new public methods backed by private
#customPropertiesMap and#declarationVariablesWeakMap, populated duringtraverse():getDeclarationVariables(declaration)— returnsvar()function nodes used in a declaration's value, including nestedvar()in fallback positions (e.g.,var(--a, var(--b)))getClosestVariableValue(func)— returns the closest value for avar()node. Resolution priority per RFC: same-block declaration → fallback → other-block declarations (hoisted) →@propertyinitial-value →undefinedgetVariableValues(func)— returns all known values:@propertyinitial-value, then declarations in source order, then fallbackNested
var()calls in fallback positions are handled by#extractNestedVarRefs, which re-parsesRawfallback text nodes (the CSS parser represents fallbacks as raw text) and tracks any nestedvar()references with correctly adjusted source locations.no-invalid-properties— refactored to use new APIgetClosestVariableValue()for primary resolution instead of maintaining its own variable mapgetClosestVariableValue()which has access to all declarations collected duringCSSSourceCode.traverse()var()refsvar()has a valid fallback, the fallback is now preferred over an invalid value from another selector blockTests
CSSSourceCodemethods (same-block, fallback priority, hoisting,@property, multiple declarations, cross-declaration isolation, nestedvar()in fallback positions including deep nesting)no-invalid-properties💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.