Make sure that Rollup's dynamicRequireTargets are included
#12839
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolo-ribaudo
added
PR: Bug Fix 🐛
nicolo-ribaudo
commented
Feb 21, 2021
| @@ -312,7 +314,7 @@ function buildRollup(packages, targetBrowsers) { | |||
| "./packages/babel-helper-create-regexp-features-plugin", | |||
| "regexpu-core", | |||
| "regenerate-unicode-properties" | |||
| ) + "/../**", | |||
| ) + "/**/*.js", | |||
There was a problem hiding this comment.
Without the explicit .js extension it was trying to parse the LICENSE file as JS.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/41632/ |
nicolo-ribaudo
commented
Feb 21, 2021
nicolo-ribaudo
added
PR: Internal 🏠
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 67d3061:
|
JLHwung
reviewed
Feb 21, 2021
nicolo-ribaudo
force-pushed
the
fix-standalone-unicode-properties-again
branch
from
0e918d0
to
cab0315
Compare
nicolo-ribaudo
force-pushed
the
fix-standalone-unicode-properties-again
branch
from
cab0315
to
00fc143
Compare
nicolo-ribaudo
force-pushed
the
fix-standalone-unicode-properties-again
branch
from
00fc143
to
1e4e597
Compare
Co-authored-by: Brian Ng <bng412@gmail.com>
This was referenced Mar 6, 2021
Closed
This was referenced Mar 12, 2021
Merged
Closed
Merged
Bump @babel/plugin-proposal-class-properties from 7.12.1 to 7.13.0 in /desktop
vamshi9666/flipper#72
Closed
Merged
Merged
Merged
github-actions
bot
added
the
outdated
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up to #12819: that PR was going in the good direction, but it didn't actually fix the issue: Rollup wasn't injecting the
dynamicRequireTargetsanywhere, and we didn't notice it because apparently it fallbacks to Node'srequire()function (which works on CI). You can see it in #12819's repl.I noticed it when rebasing #12015: that PR needed another
dynamicRequireTarget, but it caught the error because it was for a relative path which made Node's resolution algorithm fail.I particularly dislike this solution, but it's the only thing I could came up with to make it work.
I'd like to include this in the next version, since it's a follow-up to #12819 which has already been merged.
Repl for this PR: https://babeljs.io/repl/build/41352/#?browsers=%3E0.5%25%2C%20not%20ie%2010&build=&builtIns=false&spec=false&loose=false&code_lz=G4QwTgBAHhC8EHoA6AHA3gGQL4IK4G4g&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=false&presets=env&prettier=false&targets=&version=7.12.18%2Bpr.12839&externalPlugins=