-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Only import types from declared dependencies #16494
Conversation
cc21a3b
to
1519a1b
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57017 |
d550a2f
to
f3fd184
Compare
@@ -1,3 +1,4 @@ | |||
// eslint-disable-next-line import/no-extraneous-dependencies -- TODO: Avoid cycle |
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.
There are 4 packages whose types circularly depend on @babel/traverse
. Lets just ignore those errors for now (@babel/traverse
will be in the dependencies anyway), but I will think of a better solution before going stable.
Fixes #1, Fixes #2
This PR makes sure that all our
import type
s only reference packages that are listed independencies
/peerDependencies
. This is done mainly in two ways:@babel/core
, so they can importNodePath
/Visitor
/Scope
/types
from there rather than from@babel/traverse
/@babel/types
@babel/traverse
/@babel/types
: if a helper expects aNodePath
as a parameter, you already have to have@babel/traverse
/@babel/types
in your dependencies so this is not causing extra deps.To (2) there are some exceptions:
babel-helper-builder-react-jsx
andbabel-helper-plugin-utils
only make sense when using with a full plugin (and not directly with parser+traverse), so I added@babel/core
as a peer dependency (but only for Babel 8)babel-helper-fixtures
is also only needed when testing plugins, so I added@babel/core
as a peer depThis PR doesn't lint
@babel/standalone
and@babel/parser
since those are bundled, but we should probably add@babel/types
as a dependency of@babel/parser
. I don't really like doing it since it's perfectly reasonable to use@babel/parser
without also using@babel/types
, but on the other hand if the dependency is only used inimport type
it won't actually affect anything at runtime (it won't be loaded) so adding it is not too bad.There is one remaining lint error fixed by #16493.