Support at-rule property definitions in variable renaming#88
Support at-rule property definitions in variable renaming#88nex3 merged 5 commits intogoogle:masterfrom
Conversation
nex3
left a comment
nex3
left a comment
There was a problem hiding this comment.
Thanks for working on this! A few relatively small changes and I think we can land it.
src/variable.ts
Outdated
| atRuleNode.walkDecls(declarationNode => { | ||
| renameDeclaration(declarationNode); | ||
| }); |
There was a problem hiding this comment.
I don't think this is necessary, because it's never valid to refer to variables within a @property declaration.
There was a problem hiding this comment.
Thanks for the guidance! I initially handled this case to cover broader plugin scenarios, referencing the example in the related issue. I've simplified it now, and it turns out the refined version alone is still enough to implicitly support variable references in that example.
test/variable.test.ts
Outdated
|
|
||
| describe('with @property (contains initial-value)', () => { | ||
| const input = | ||
| '@property --test-property { syntax: "<angle>"; initial-value: var(--initial); }'; |
There was a problem hiding this comment.
This isn't a valid example for a couple reasons: not only can you not reference a variable in initial-value, a @property rule must contain inherits in order to not be discarded. We should probably make all examples run on valid @property rules.
Also, as a style nit, use a multi-line string if the string doesn't fit within 80 columns.
There was a problem hiding this comment.
Thanks for the clarification! I've updated the examples and the formatting.
By the way, Prettier suggests formatting for a few unrelated files:
.github/workflows/ci.ymlCHANGELOG.mdREADME.md
Should we apply those changes before the CI check?
There was a problem hiding this comment.
Anything that's not covered by gts check I'm not particularly interested in keeping formatted to Prettier's standards.
| @property --test-property { | ||
| syntax: "<angle>"; | ||
| inherits: false; | ||
| }`; |
There was a problem hiding this comment.
Either set syntax: "*" or give this an initial value; otherwise it's invalid per spec.
There was a problem hiding this comment.
Updated to syntax: "*".
2 participants
This PR adds support for renaming variables in CSS
@propertydefinitions. Resolves #82.To maintain consistency with the existing codebase, I've implemented an
AtRule.propertyprocessor using:renameVariablefor property namesrenameDeclarationfor internal declarationsI have added test cases to verify the implementation across all core strategies:
nonedebugminimal(including both declaration-first and property-first orderings)If any further adjustments are needed, please let me know.