-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Automatically apply babel-loader for apps with babel configs #3862
Conversation
This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Socket Security Pull Request Report👍 No new dependency issues detected in pull request Pull request report summary
Bot CommandsTo ignore an alert, reply with a comment starting with Powered by socket.dev |
|
Benchmark for b730c7eClick to view benchmark
|
Benchmark for 4c7aa1d
Click to view full benchmark
|
Benchmark for 2c599adClick to view benchmark
|
let has_babel_config = { | ||
let mut has_babel_config = false; | ||
for filename in BABEL_CONFIG_FILES { | ||
let metadata = project_root.join(filename).metadata().await; |
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.
let metadata = project_root.join(filename).metadata().await; | |
let metadata = project_root.join(filename).get_type().await; |
We usually use get_type for existence checks as it's cheaper as it doesn't require a syscall
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's also find_context_file
which does tree lookups.
RequestVc::parse(Value::new(Pattern::Constant( | ||
"babel-loader/package.json".to_string(), | ||
))), | ||
resolve_options( |
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 node_resolve_options somewhere in our repo. You can use them.
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.
See assert_can_resolve_react_refresh
let has_babel_config = { | ||
let mut has_babel_config = false; | ||
for filename in BABEL_CONFIG_FILES { | ||
let metadata = project_root.join(filename).metadata().await; |
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's also find_context_file
which does tree lookups.
|
||
if has_babel_config { | ||
let mut options = (*webpack_options.await?).clone(); | ||
for ext in [".js", ".jsx", ".ts", ".tsx", ".cjs", ".mjs"] { |
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.
Should this be using next_config.page_extensions()
instead? Also, what about .cts
, .mts
, .ctsx
, and .mtsx
?
} | ||
.cell() | ||
.as_issue() | ||
.emit() |
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.
Is this going to be emitted for every extension?
RequestVc::parse(Value::new(Pattern::Constant( | ||
"babel-loader/package.json".to_string(), | ||
))), | ||
resolve_options( |
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.
See assert_can_resolve_react_refresh
Benchmark for 7380915
Click to view full benchmark
|
This: * Addresses feedback from #3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these
This: * Addresses feedback from #3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
…turborepo#3862) This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
This: * Addresses feedback from vercel/turborepo#3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
…turborepo#3862) This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
This: * Addresses feedback from vercel/turborepo#3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
…turborepo#3862) This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
This: * Addresses feedback from vercel/turborepo#3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
…turborepo#3862) This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
This: * Addresses feedback from vercel/turborepo#3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
…turborepo#3862) This will automatically apply `babel-loader` to Next.js apps that include a babel configuration file alongside `next.config.js`. It requires the app to provide `babel-loader` itself. Test Plan: Added an integration test.
This: * Addresses feedback from vercel/turborepo#3862 * Exempts VirtualAssets from webpack loaders. This addresses a bug where next-hydrate.tsx could not be read. * Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these --------- Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
This will automatically apply
babel-loader
to Next.js apps that include a babel configuration file alongsidenext.config.js
. It requires the app to providebabel-loader
itself.Test Plan: Added an integration test.