[eslint-plugin] support validImports options for all rules #1085
Conversation
| @@ -24,11 +24,17 @@ const invalidExportWithDefineVars = | |||
| ruleTester.run('stylex-enforce-extension', rule.default, { | |||
| valid: [ | |||
| { | |||
| code: 'export const vars = stylex.defineVars({});', | |||
| code: ` | |||
| import stylex from 'stylex'; | |||
There was a problem hiding this comment.
The sideeffect of createImportTracker refactoring is that import statements must now be present in the file, not just specifying it in validImports option as we now collect import statements and check against the collected values.
Technically a breaking change but should not affect real code as imports should exist anyway.
| 'ExportNamedDeclaration, ExportDefaultDeclaration'(node: Node): void { | ||
| checkExports(node); | ||
| reportErrors(node); | ||
| }, | ||
| 'Program:exit'() { | ||
| importTracker.clear(); |
There was a problem hiding this comment.
I'm not too sure this cleanup is strictly necessary, but aligned it with packages/@stylexjs/eslint-plugin/src/stylex-sort-keys.js which does clean up.
| .forEach(parseStylexImportStyle); | ||
| // stylex.create can only be singular variable declarations at the root | ||
| // of the file so we can look directly on Program and populate our set. | ||
| .filter( |
There was a problem hiding this comment.
we need to ensure we collect import statements before variable detection happens, so this can't happen in ImportDeclaration and needs to be done in Program
| } | ||
|
|
||
| return { | ||
| ImportDeclaration: handleImportDeclaration, |
There was a problem hiding this comment.
Because of the conflict with ImportDeclaration type, function name cannot be ImportDeclaration so it was named handleImportDeclaration and aliased as ImportDeclaration here.
| @@ -16,7 +16,7 @@ sidebar_position: 2 | |||
| type Options = { | |||
| // Possible strings where you can import stylex from | |||
| // | |||
| // Default: ['@stylexjs/stylex'] | |||
| // Default: ['stylex', '@stylexjs/stylex'] | |||
There was a problem hiding this comment.
Existing code already had stylex in validImports by default so I aligned the default for all rules and reflected it in the docs too.
packages/@stylexjs/eslint-plugin/__tests__/stylex-enforce-extension-test.js
Outdated
Show resolved
Hide resolved
packages/@stylexjs/eslint-plugin/__tests__/stylex-enforce-extension-test.js
Outdated
Show resolved
Hide resolved
packages/@stylexjs/eslint-plugin/__tests__/stylex-enforce-extension-test.js
Outdated
Show resolved
Hide resolved
packages/@stylexjs/eslint-plugin/src/stylex-no-legacy-contextual-styles.js
Outdated
Show resolved
Hide resolved
bee2b7e to
8985f91
Compare
What changed / motivation ?
Continuing on #1039, this PR adds
validImportsoption to all eslint rules with support for both the string and object syntax uniformly, for RSD compatibility and other custom aliases.I've also updated API docs and the README to document the options. I expect documentation rework to be made in another PR (pending #1057) so made the change minimal.
Main changes:
validImportsoption to all rulessrc/utils/createImportTracker.jsto reduce duplicationLinked PR/Issues
Fixes #1056
Additional Context
I ran the following tests locally and confirmed existing and new tests are passing:
npm run testScreenshots, Tests, Anything Else
Pre-flight checklist
Contribution Guidelines