ESLint rule to validate css props are declared before{...spread} props
#6257
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6257/ |
1Copenut
added
skip-changelog
work in progress
testing
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6257/ |
…ing." * Updating unit tests to pass before renaming. * Registering ESLint rule locally before adding unit tests. * Switched to JSXElement, and array of arrays. * Simplified logic, added unit tests, fixed failures.
| return { | ||
| JSXElement(node) { | ||
| const tempArr = []; | ||
| node.openingElement.attributes.forEach(attribute => tempArr.push(attribute)); |
There was a problem hiding this comment.
@constancecchen Your comment about making an array of arrays was spot on. It took a few iterations, but I found good results removing all extra logic and running our previous checks against each array in turn.
| }, | ||
| ]; | ||
|
|
||
| const invalid = [ |
There was a problem hiding this comment.
Each invalid block was built out from real failures in EUI components, or derived therefrom.
1Copenut
changed the title
css props are declared before{...spread} props
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6257/ |
|
I'll look over the components with linting changes for visual regression. LMK if this feels like a breaking change and I'll add the label. |
|
This looks and works great. I just have a couple very minor comments, fantastic work on this Trevor! edit to add: this is not a breaking change, just IMO, and I agree it doesn't need a changelog |
|
Thank you @constancecchen for the 💯 code change suggestions. I turned on Prettier format on save, so it broke some of my one line I esp liked the suggestion to move early return guard clauses closer to their declarations. Moving those lines around provided clearer intent. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6257/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6257/ |
Summary
Adding an ESLint rule to validate
css={cssProps}are added before{...rest}spread props in our EUI components.Here's a neat screen shot showing the test running against unit tests. Each unit test was derived from an actual failure while linting locally.