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
Bundle most packages in Babel 8 #14179
Bundle most packages in Babel 8 #14179
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52958/ |
5ab46bb
to
e3aa0e5
Compare
@@ -393,8 +405,33 @@ function buildRollup(packages, targetBrowsers) { | |||
name, | |||
sourcemap: sourcemap, | |||
exports: "named", | |||
interop(id) { |
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.
The interop
config fixes a regression issue: Using @babel/plugin-proposal-optional-chaining
with an old @babel/parser
version will throw optional chaining syntax is required.
In https://unpkg.com/@babel/plugin-proposal-optional-chaining@7.16.7/lib/index.js,
inherits: syntaxOptionalChaining__default['default'].default,
the inherited syntax plugin is always undefined
because we and rollup both apply interop transform on the default exports.
Now it generates
inherits: syntaxOptionalChaining.default,
as expected.
1e618f9
to
e9f9361
Compare
} | ||
if (pkgJSON === undefined) continue; | ||
const src = packageDir + "/" + dir; | ||
if (pkgJSON.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.
We may extend support to pkgJSON.exports
in the future. However, bundling a package with multiple exports could be tricky: Do we want to bundle multiple entry thus minimizing internal module imports? Or do we want to bundle modules in topological order thus minimizing the artifact size?
Since few packages supports multiple exports, I think supporting main
is good enough.
e9f9361
to
df66712
Compare
11e6b20
to
61d987f
Compare
The CI error is unrelated, it will be fixed in #14087. |
61d987f
to
f1ac928
Compare
0822478
to
5c6bae9
Compare
To be honest, I'm a little concerned that this will affect debugging while in use, hope it's ok. |
Note that |
Yeah, I mean when babel is a dependency. PS: I haven't even used |
Also it looks like this PR is done #9575. |
@@ -294,23 +299,49 @@ if (process.env.CIRCLE_PR_NUMBER) { | |||
|
|||
const babelVersion = | |||
require("./packages/babel-core/package.json").version + versionSuffix; | |||
function buildRollup(packages, targetBrowsers) { | |||
function buildRollup(packages, buildStandalone) { | |||
const sourcemap = process.env.NODE_ENV === "production"; |
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.
Not related to this PR:
Do we need to include source maps in the release?
Currently NODE_ENV
is only used in prepublish-build
(doesn't even include prepublish-build-standalone
), I don't know if this is expected.
(found this when I tried to debug in the repl)
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.
Good catch. I think the source maps should be included, especially for a browser package like babel-standalone
.
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
a090d08
to
a8e086e
Compare
BABEL_8_BREAKING
flagIn this PR we bundle most packages when built with
BABEL_8_BREAKING
with exceptions listed below:eslint/*
packages@babel/runtime
@babel/runtime-corejs2
@babel/runtime-corejs3
@babel/cli
: Rollup hangs due to shebang@rollup/plugin-node-resolve
does not supportbrowser: false
whenexports
are provided in package.json@babel/register
@babel/core
@babel/plugin-transform-runtime
@babel/helper-fixtures
: `@rollup/plugin-commonjs messes up with dynamic require in this package@babel/helpers
: Some fixture tests requires internal lib/helpers accessNotable internal changes:
@babel/traverse
:@babel/traverse
. Instead the typings are now explicitly listed onpackages/babel-traverse/src/path/lib/virtual-types.ts
andpackages/babel-traverse/src/path/lib/virtual-types-validator.ts
.t["is" + virtualType] = validators[virtualType].checkPath
, they are now explicitly declared inpackages/babel-traverse/src/path/lib/virtual-types-validator.ts
. Now TS manages to detect some bugs in the validators and I have fixed reported issues. And it improves method navigability.Known issues: I will fix them in a subsequent PR so the Babel-traverse / optional chaining fixes in this PR can be landed to Babel 7 soon.
react-jsx-development
depends onexports
inreact-jsx
corejs-compat/data.json
is bundled inpreset-env
Because the bundling happens during prepublish phrase, it can only be tested by Babel 8 breaking tests hosted in Circle CI.