-
Notifications
You must be signed in to change notification settings - Fork 225
chore: import/no-extraneous-dependencies off for stories and test files #5481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
packages/.eslintrc.json
Outdated
"**/test/**/*.test.{js,ts,jsx,tsx}", | ||
"**/stories/**/*.{js,ts,jsx,tsx}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great but do we want to scope these a little more?
"**/test/**/*.test.{js,ts,jsx,tsx}", | |
"**/stories/**/*.{js,ts,jsx,tsx}" | |
"{packages,tools}/*/test/*.test.ts", | |
"{packages,tools}/*/stories/*.ts" |
To make sure we're not ignoring assets we want to validate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a suggestion about making the selectors more specific.
packages/.eslintrc.json
Outdated
@@ -83,6 +83,15 @@ | |||
] | |||
}, | |||
"overrides": [ | |||
{ | |||
"files": [ | |||
"{packages,tools}/*/test/*.test.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: what if we made these agnostic so we don't miss any rogue files in the repo?
"{packages,tools}/*/test/*.test.ts", | |
"**/test/*.test.ts", | |
"**/stories/*.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rajdeepc reopened this as tools has no context inside of packages. I recommend keeping the stars and adding the same rule to tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid confusion, I have added the check to switch off it under
"files": [
"*.test.ts",
"*.stories.ts",
"**/benchmark/*.ts",
"**/test/*.ts"
],
@Rajdeepc I think during cutover I needed to add this rule in to get around linter errors in script testing. Can you confirm that's correct in main (that's what I see in the conflict). If it is correct and included in main, we can close this with a comment calling out the cutover PR resolved this. |
Description
This PR modifies the ESLint configuration to disable the
import/no-extraneous-dependencies
rule for test and storybook files, allowing these files to import development dependencies without triggering linting errors.Changes
.eslintrc.json
to turn offimport/no-extraneous-dependencies
rule for:**/test/**/*.test.{js,ts,jsx,tsx}
)**/stories/**/*.{js,ts,jsx,tsx}
)Impact
This change:
Motivation and context
Related issue(s)
Screenshots (if appropriate)
Author's checklist
Reviewer's checklist
patch
,minor
, ormajor
featuresManual review test cases
Create a test commit in your local branch or in this branch
Device review