-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
esbuild: prioritize "module" over "browser" #37052
Conversation
@@ -456,6 +456,7 @@ async function esbuildCompile(srcDir, srcFilename, destDir, options) { | |||
incremental: !!options.watch, | |||
logLevel: 'silent', | |||
external: options.externalDependencies, | |||
mainFields: ['module', 'browser', 'main'], |
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's update the test running with this, too.
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.
What test?
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.
All of them:
amphtml/build-system/tasks/runtime-test/runtime-test-base.js
Lines 232 to 240 in 31cf6fd
this.esbuild = { | |
target: 'es5', | |
define: { | |
'process.env.NODE_DEBUG': 'false', | |
'process.env.NODE_ENV': '"test"', | |
}, | |
plugins: [importPathPlugin, babelPlugin], | |
sourcemap: 'inline', | |
}; |
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.
Ah, forgot about this. Sure
summary
Partial for #36453
By default esbuild will utilize browser, module, main in that order (docs). This PR swaps the priorities of module/browser for better tree shaking of our dependencies. In particular, this allows for the elision of the cache-url package.
size diff