Fix babel-standalone for realz #6137

Daniel15 · 2017-08-19T23:32:48Z

Q	A
Fixed Issues	Closes #6132, Closes #6135, References #6120
Patch: Bug Fix?	Yes
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added/Pass?	N/A / Yes
Spec Compliancy?	N/A
License	MIT
Doc PR	No
Any Dependency Changes?

The root package.json has a reference to babel-loader and babel-core. The root babel-core comes from npm (for some reason) rather than the version in the repo itself, which causes odd issues. By installing babel-loader into babel-standalone's node_modules directory rather than the root node_modules directory, it picks up the correct version of babel-core and everything works fine.

Also made gulpTasks.js properly modular so babel-minify can reuse it.

Huge thanks to @jridgewell for the initial investigation into this issue.

mention-bot · 2017-08-19T23:32:55Z

@Daniel15, thanks for your PR! By analyzing the history of the files in this pull request, we identified @existentialism, @hzoo and @ChauTNguyen to be potential reviewers.

jridgewell · 2017-08-20T00:24:47Z

This doesn't seem to fix it. I still get a babel-loader@7.1.1 in the monorepo's node_modules, which causes the same issue.

Daniel15 · 2017-08-20T00:27:18Z

Hmm, that's interesting @jridgewell... I wonder why it's working for me locally? I even rebased on top of your PR and it built fine.

I'll keep digging, I might just have to hack the module resolution.

Daniel15 · 2017-08-20T02:01:35Z

@jridgewell - Are you sure you ran make bootstrap and make build? I ran make bootstrap which totally blows away node_modules and installs the npm packages, and I do not have babel-loader in the root node_modules.

jridgewell · 2017-08-20T02:40:38Z

Yup.

 ✓ jridgewell@Dandy  ~/src/babel  class-private-properties  git log
Sat Aug 19 20:18:25 2017 -0400 ef6ea1022 (HEAD -> class-private-properties) Merge branch 'pr/6137' into class-private-properties  [Justin Ridgewell]
Sat Aug 19 17:01:15 2017 -0700 eb138bba7 (pr/6137) Fix infinite loop in Makefile (oops)  [Daniel Lo Nigro]
Sat Aug 19 16:41:05 2017 -0700 e5dcf16ab Bump Yarn version to see if it fixes the CircleCI build  [Daniel Lo Nigro]
Sat Aug 19 16:26:34 2017 -0700 16007eccd Fix babel-standalone  [Daniel Lo Nigro]
Sat Aug 19 22:35:40 2017 +0900 c6a094a9d Split export extensions into 2 different plugins, update stage presets (#6080)  [Sangboak Lee]
Fri Aug 18 23:42:39 2017 -0400 3818a64ee (origin/class-private-properties) Final fixes  [Justin Ridgewell]
 ✓ jridgewell@Dandy  ~/src/babel  class-private-properties  node -v
v8.0.0
 ✓ jridgewell@Dandy  ~/src/babel  class-private-properties  npm -v
5.3.0
 ✓ jridgewell@Dandy  ~/src/babel  class-private-properties  yarn --version
0.28.4
 ✓ jridgewell@Dandy  ~/src/babel  class-private-properties  make bootstrap
...
ERROR in ./src/index.js
Module build failed: Error: [BABEL] /Users/jridgewell/src/babel/packages/babel-standalone/src/index.js: You gave us a visitor for the node type "PrivateName" but it's not a valid type
    at verify (/Users/jridgewell/src/babel/node_modules/babel-core/node_modules/babel-traverse/lib/visitors.js:156:13)
    at Function.explode (/Users/jridgewell/src/babel/node_modules/babel-core/node_modules/babel-traverse/lib/visitors.js:59:3)
    at instantiatePlugin (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:304:27)
    at loadPluginDescriptor (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:280:14)
    at /Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:73:14
    at Array.map (native)
    at OptionManager.mergeOptions (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:72:34)
    at /Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:100:15
    at Array.forEach (native)
    at OptionManager.mergeOptions (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:99:15)
 ✘ jridgewell@Dandy  ~/src/babel  class-private-properties  ls node_modules | grep babel-loader
babel-loader

Daniel15 · 2017-08-20T03:00:04Z

Huh. Strange. I'll take another shot at fixing this. Thanks for confirming. Sent from my phone.

…

On Aug 19, 2017 7:41 PM, "Justin Ridgewell" ***@***.***> wrote: Yup. ✓ ***@***.*** ~/src/babel class-private-properties git log Sat Aug 19 20:18:25 2017 -0400 ef6ea1022 (HEAD -> class-private-properties) Merge branch 'pr/6137' into class-private-properties [Justin Ridgewell] Sat Aug 19 17:01:15 2017 -0700 eb138bb (pr/6137) Fix infinite loop in Makefile (oops) [Daniel Lo Nigro] Sat Aug 19 16:41:05 2017 -0700 e5dcf16 Bump Yarn version to see if it fixes the CircleCI build [Daniel Lo Nigro] Sat Aug 19 16:26:34 2017 -0700 16007ec Fix babel-standalone [Daniel Lo Nigro] Sat Aug 19 22:35:40 2017 +0900 c6a094a Split export extensions into 2 different plugins, update stage presets (#6080) [Sangboak Lee] Fri Aug 18 23:42:39 2017 -0400 3818a64 (origin/class-private-properties) Final fixes [Justin Ridgewell] ✓ ***@***.*** ~/src/babel class-private-properties node -v v8.0.0 ✓ ***@***.*** ~/src/babel class-private-properties npm -v 5.3.0 ✓ ***@***.*** ~/src/babel class-private-properties yarn --version 0.28.4 ✓ ***@***.*** ~/src/babel class-private-properties make bootstrap ... ERROR in ./src/index.js Module build failed: Error: [BABEL] /Users/jridgewell/src/babel/packages/babel-standalone/src/index.js: You gave us a visitor for the node type "PrivateName" but it's not a valid type at verify (/Users/jridgewell/src/babel/node_modules/babel-core/node_modules/babel-traverse/lib/visitors.js:156:13) at Function.explode (/Users/jridgewell/src/babel/node_modules/babel-core/node_modules/babel-traverse/lib/visitors.js:59:3) at instantiatePlugin (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:304:27) at loadPluginDescriptor (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:280:14) at /Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:73:14 at Array.map (native) at OptionManager.mergeOptions (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:72:34) at /Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:100:15 at Array.forEach (native) at OptionManager.mergeOptions (/Users/jridgewell/src/babel/node_modules/babel-core/lib/config/option-manager.js:99:15) ✘ ***@***.*** ~/src/babel class-private-properties ls node_modules | grep babel-loaderbabel-loader — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6137 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFnHUqV2KrKmji1riiBNMQRxsHWdQGmks5sZ5y7gaJpZM4O8eJk> .

Daniel15 · 2017-08-20T05:34:39Z

@jridgewell Could you please try the latest version and see if that works for you? To test, I rebased a copy of your branch on top of this branch, and it seems to build successfully on CircleCI. There's some test failures, not sure if they're directly related or not (seems not): https://circleci.com/gh/Daniel15/babel/29

jridgewell

Works for me, but it feels super hacky. Definitely need someone else to approve.

Daniel15 · 2017-08-22T16:38:12Z

Ping @babel/core - Are you OK with this PR?

hzoo · 2017-08-22T19:32:25Z

I guess it feels kinda bad to have to do this but it's broken atm so ok 👍

nicolo-ribaudo

Would something like this work?

# cwd = babel-core/
yarn link
cd ../babel-standalone/node_modules/babel-loader
yarn link babel-core

It still is very hacky (changing the dependencies of a dependency isn't something you see often), but it feels less hacky than overriding node's module resolution algorithm.

Anyway, this is just internal code: it doesn't need to be best possible.

nicolo-ribaudo · 2017-08-22T20:05:34Z

packages/babel-standalone/src/overrideModuleResolution.js

+ * override Node's module resolution algorithm to specify a custom resolver for
+ * babel-core to *force* it to use our version.
+ *
+ * Here be dragons.


(You could add an emoji! 🐉 🐲 )

I'm always worried that inserting Emoji into code is going to break something somewhere 😛

Daniel15 · 2017-08-22T20:45:36Z

@nicolo-ribaudo

Would something like this work?

It wouldn't :( The version at babel-standalone/node_packages/babel-core is the correct version, but the version at the root node_modules is the wrong one, and babel-loader is being hoisted to the root. We can likely run yarn link in the root but I was worried that'd break stuff.

This was referenced Aug 19, 2017

Class private properties #6120

Closed

Remove babel-standalone #6135

Closed

Daniel15 mentioned this pull request Aug 20, 2017

Allow nightly Yarn builds to be used #6138

Merged

Daniel15 added the WIP label Aug 20, 2017

jridgewell approved these changes Aug 20, 2017

View reviewed changes

Daniel15 removed the WIP label Aug 20, 2017

Daniel15 added 3 commits August 20, 2017 16:20

Fix babel-standalone

523bc24

Fix infinite loop in Makefile (oops)

1f5cc1c

Override Node.js module resolution to handle babel-core

5b9722c

Daniel15 force-pushed the fix-it-fix-it-fix-it-fix-it branch from 974ffe4 to 5b9722c Compare August 20, 2017 23:21

hzoo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Aug 22, 2017

nicolo-ribaudo reviewed Aug 22, 2017

View reviewed changes

Daniel15 merged commit 93cf26a into babel:7.0 Aug 22, 2017

Daniel15 deleted the fix-it-fix-it-fix-it-fix-it branch August 22, 2017 20:46

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix babel-standalone for realz #6137

Fix babel-standalone for realz #6137

Daniel15 commented Aug 19, 2017

mention-bot commented Aug 19, 2017

jridgewell commented Aug 20, 2017

Daniel15 commented Aug 20, 2017

Daniel15 commented Aug 20, 2017

jridgewell commented Aug 20, 2017

Daniel15 commented Aug 20, 2017 via email

Daniel15 commented Aug 20, 2017 •

edited

Loading

jridgewell left a comment

Daniel15 commented Aug 22, 2017

hzoo commented Aug 22, 2017

nicolo-ribaudo left a comment

nicolo-ribaudo Aug 22, 2017

Daniel15 Aug 22, 2017

Daniel15 commented Aug 22, 2017 •

edited

Loading

Fix babel-standalone for realz #6137

Fix babel-standalone for realz #6137

Conversation

Daniel15 commented Aug 19, 2017

mention-bot commented Aug 19, 2017

jridgewell commented Aug 20, 2017

Daniel15 commented Aug 20, 2017

Daniel15 commented Aug 20, 2017

jridgewell commented Aug 20, 2017

Daniel15 commented Aug 20, 2017 via email

Daniel15 commented Aug 20, 2017 • edited Loading

jridgewell left a comment

Choose a reason for hiding this comment

Daniel15 commented Aug 22, 2017

hzoo commented Aug 22, 2017

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo Aug 22, 2017

Choose a reason for hiding this comment

Daniel15 Aug 22, 2017

Choose a reason for hiding this comment

Daniel15 commented Aug 22, 2017 • edited Loading

Daniel15 commented Aug 20, 2017 •

edited

Loading

Daniel15 commented Aug 22, 2017 •

edited

Loading