-
Notifications
You must be signed in to change notification settings - Fork 46
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
front: enable import/no-extraneous-dependencies lint #10707
Conversation
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.
Overall nice changes, just a couple small issues.
(Perhaps it would be a good idea to have a short meeting at some point to more generally review and update the linting rules we are using)
Don't use the .tsx file extension, because there's no JSX in there. While at it, use .spec.ts instead of .test.ts since it's the more widely used suffix. Signed-off-by: Simon Ser <[email protected]>
We don't use moment anywhere. Signed-off-by: Simon Ser <[email protected]>
This is the pattern used everywhere else. Signed-off-by: Simon Ser <[email protected]>
fail() is a jest-ism. We need to use assert.fail() with vitest. Fixes the following error when @types/jest is dropped: /home/simon/src/osrd/front/src/common/IntervalsDataViz/data.spec.ts 272:9 warning Unsafe call of a(n) `error` type typed value @typescript-eslint/no-unsafe-call 287:9 warning Unsafe call of a(n) `error` type typed value @typescript-eslint/no-unsafe-call 302:9 warning Unsafe call of a(n) `error` type typed value @typescript-eslint/no-unsafe-call Signed-off-by: Simon Ser <[email protected]>
We use vitest for testing, not jest. Remove all remaining references to jest. Signed-off-by: Simon Ser <[email protected]>
This is a valuable lint to figure out cases where we're missing a dependency (and only get it by chance because another dependency pulls it in). Signed-off-by: Simon Ser <[email protected]>
219cd85
to
52f6d6d
Compare
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.
LGTM ^^
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.
LGTM, thanks for this PR 🙏
This is a valuable lint to figure out cases where we're missing a
dependency (and only get it by chance because another dependency
pulls it in).
See individual commits.