Skip to content
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

Make sure that Rollup's dynamicRequireTargets are included #12839

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions .yarn/patches/@rollup__plugin-commonjs.patch
@@ -0,0 +1,15 @@
See packages/babel-standalone/src/dynamic-require-entrypoint.cjs for the reason for this diff.

diff --git a/dist/index.js b/dist/index.js
index 712f6a7d81b115d468a764b4139caa00d6cfc358..73fbf004217f3d44b6420d3082a0846b53e00f4c 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -1626,7 +1626,7 @@ function commonjs(options = {}) {
const sourceMap = options.sourceMap !== false;

function transformAndCheckExports(code, id) {
- if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) {
+ if (isDynamicRequireModulesEnabled && (this.getModuleInfo(id).isEntry || id.endsWith("/dynamic-require-entrypoint.cjs"))) {
code =
getDynamicPackagesEntryIntro(dynamicRequireModuleDirPaths, dynamicRequireModuleSet) + code;
}
8 changes: 5 additions & 3 deletions Gulpfile.mjs
Expand Up @@ -245,8 +245,8 @@ function resolveChain(baseUrl, ...packages) {

return packages.reduce(
(base, pkg) =>
require.resolve(pkg + "/package.json", { paths: [path.dirname(base)] }),
fileURLToPath(baseUrl)
path.dirname(require.resolve(pkg + "/package.json", { paths: [base] })),
path.dirname(fileURLToPath(baseUrl))
);
}

Expand Down Expand Up @@ -304,6 +304,8 @@ function buildRollup(packages, targetBrowsers) {
// Rollup doesn't read export maps, so it loads the cjs fallback
"packages/babel-compat-data/*.js",
"packages/*/src/**/*.cjs",
// See the comment in this file for the reason to include it
"packages/babel-standalone/src/dynamic-require-entrypoint.cjs",
],
dynamicRequireTargets: [
// https://github.com/mathiasbynens/regexpu-core/blob/ffd8fff2e31f4597f6fdfee75d5ac1c5c8111ec3/rewrite-pattern.js#L48
Expand All @@ -312,7 +314,7 @@ function buildRollup(packages, targetBrowsers) {
"./packages/babel-helper-create-regexp-features-plugin",
"regexpu-core",
"regenerate-unicode-properties"
) + "/../**",
) + "/**/*.js",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the explicit .js extension it was trying to parse the LICENSE file as JS.

],
}),
rollupBabel({
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -32,7 +32,7 @@
"@babel/register": "^7.12.0",
"@babel/runtime": "^7.12.0",
"@rollup/plugin-babel": "^5.2.0",
"@rollup/plugin-commonjs": "^17.1.0",
"@rollup/plugin-commonjs": "patch:@rollup/plugin-commonjs@^17.1.0#./.yarn/patches/@rollup__plugin-commonjs.patch",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^9.0.0",
"@rollup/plugin-replace": "^2.3.3",
Expand Down
13 changes: 13 additions & 0 deletions packages/babel-standalone/src/dynamic-require-entrypoint.cjs
@@ -0,0 +1,13 @@
/*
We bundle @babel/standalone using the dynamicRequireTargets option.
This option needs to inject requrie() calls to all the modules that
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved
could be dynamically required later.

By default it injects them in the entrypoint, but when rollup loads the
entrypoint it finds syntax that it doesn't support (because
@rollup/plugin-commonjs must run before @rollup/plugin-babel).

We use "yarn patch" to modify the @rollup/plugin-commonjs package, so
that it injects those "preload" require() calls in this file rather
than in the entrypoint.
*/
3 changes: 3 additions & 0 deletions packages/babel-standalone/src/index.js
Expand Up @@ -10,6 +10,9 @@
/* global VERSION */
/* eslint-disable max-len */

// $FlowIgnore
import "./dynamic-require-entrypoint.cjs";
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved

import {
transformFromAst as babelTransformFromAst,
transform as babelTransform,
Expand Down
21 changes: 19 additions & 2 deletions yarn.lock
Expand Up @@ -3703,7 +3703,7 @@ __metadata:
languageName: node
linkType: hard

"@rollup/plugin-commonjs@npm:^17.1.0":
"@rollup/plugin-commonjs@^17.1.0":
version: 17.1.0
resolution: "@rollup/plugin-commonjs@npm:17.1.0"
dependencies:
Expand All @@ -3720,6 +3720,23 @@ __metadata:
languageName: node
linkType: hard

"@rollup/plugin-commonjs@patch:@rollup/plugin-commonjs@^17.1.0#./.yarn/patches/@rollup__plugin-commonjs.patch::locator=babel%40workspace%3A.":
version: 17.1.0
resolution: "@rollup/plugin-commonjs@patch:@rollup/plugin-commonjs@npm%3A17.1.0#./.yarn/patches/@rollup__plugin-commonjs.patch::version=17.1.0&hash=a0d076&locator=babel%40workspace%3A."
dependencies:
"@rollup/pluginutils": ^3.1.0
commondir: ^1.0.1
estree-walker: ^2.0.1
glob: ^7.1.6
is-reference: ^1.2.1
magic-string: ^0.25.7
resolve: ^1.17.0
peerDependencies:
rollup: ^2.30.0
checksum: ccf44f300b5b7b9fcc72e22c414bd86ce461e412a0980fb2f73d6d2b562466409ecaae270a54028c4584650c7d1a0718a6af0d15c5a5b41d6cd20a2159ba34fb
languageName: node
linkType: hard

"@rollup/plugin-json@npm:^4.1.0":
version: 4.1.0
resolution: "@rollup/plugin-json@npm:4.1.0"
Expand Down Expand Up @@ -4857,7 +4874,7 @@ __metadata:
"@babel/register": ^7.12.0
"@babel/runtime": ^7.12.0
"@rollup/plugin-babel": ^5.2.0
"@rollup/plugin-commonjs": ^17.1.0
"@rollup/plugin-commonjs": "patch:@rollup/plugin-commonjs@^17.1.0#./.yarn/patches/@rollup__plugin-commonjs.patch"
"@rollup/plugin-json": ^4.1.0
"@rollup/plugin-node-resolve": ^9.0.0
"@rollup/plugin-replace": ^2.3.3
Expand Down