-
Notifications
You must be signed in to change notification settings - Fork 0
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
refactor: remove eslint-config-airbnb* dependencies #160
Conversation
986c35e
to
d60fdcb
Compare
As @lixiaoyan suggests, this PR will be separated to more PRs for easier reviewing and smoother updates for downstream. |
ec2488e
to
9e19cad
Compare
9e19cad
to
0c68565
Compare
"@types/confusing-browser-globals": "1.0.3", | ||
"@types/semver": "7.5.8" |
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.
Consider moving these two dependencies to dependencies
section?
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.
We are not reexporting these types; It is not very necessary to do so IMO
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.
cc @liby What do you think ? 👀
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.
Personally, I would prefer to have the package and its type definition in the same section.
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.
Moving type definition packages to dependencies
can cause unnecessary bloat and version conflicts in production. It's generally better to keep them in devDependencies
to avoid these issues, unless runtime type definitions are specifically needed. This keeps the dependency tree cleaner and more manageable.
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.
After discussion, moving @types/*
to dependencies
does not have any obvious issues. This is more like a style preference issue:
-
I prefer to put type packages that are not exported in
devDependencies
. -
I prefer to put the type package and its package together.
This reminds me of discussions about indentation, and if we want to choose a style preference, we can only vote within the entire team 🥹
packages/eslint-config-typescript-react/src/rules/react-a11y.ts
Outdated
Show resolved
Hide resolved
0c68565
to
ef80176
Compare
This PR resolves the step 1 of #159