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

rootMode: 'upward' for @babel/register? #54

Closed
ljqx opened this issue Dec 11, 2018 · 10 comments · Fixed by #58
Closed

rootMode: 'upward' for @babel/register? #54

ljqx opened this issue Dec 11, 2018 · 10 comments · Fixed by #58

Comments

@ljqx
Copy link

ljqx commented Dec 11, 2018

Hi, Gulp team,

We are upgrading Babel to 7. And as we were using gulp.babel.js, so now we are moving from babel/register to @babel/register too.

But Babel 7 has a different way to find the config file.

As we are using a monorepo, now inside subpackage, we cannot run gulp anymore as @babel/register cannot find the babel config file outside of cwd.

So I think it's better here interpret can set rootMode: 'upward' as suggested by Babel team to keep same experience as before. Now as the user of gulp we have no access of @babel/register options so seems here it's the best place to do the fix.

Thanks,

@phated
Copy link
Member

phated commented Dec 11, 2018

hm, I think this would be a breaking change for people that were expecting only their root babel.config.js to take effect.

Thoughts?

@ljqx
Copy link
Author

ljqx commented Dec 12, 2018

I think no, as the doc says, upward works as

... will make Babel search from the working directory upward looking for your babel.config.js file, and will use its location as the "root" value.

So there stills only one babel.config.js is working. The change is

  • now, it would only search for the one in cwd. If there is no such config, then it's empty.
  • after, it would search upward, if there is no such config in cwd, it would search in parent folders. Actually this is pretty similar as before in babel/register or babel-core/register. And this is same as gulp to search for gulpfile.*

So for the people that were expecting only their cwd babel.config.js to take effect, it's still the same.

The change happens for the people (us) who don't have babel.config.js in their cwd, at same time have one in parent folders, but as now Babel is not working, why would they use gulpfile.babel.js?

@phated
Copy link
Member

phated commented Dec 12, 2018

If it "stops" at the first found babel.config.js, then that should be okay. I thought it would do merging. Can you confirm with the babel team that it won't be merged?

@ljqx
Copy link
Author

ljqx commented Dec 12, 2018

Maybe it's more reliable with code?
Check: https://github.com/ljqx/babel-register-test
After yarn

  • yarn run-in-root would print default which means the code is compiled with @babel/plugin-proposal-nullish-coalescing-operator which is defined in the root babel.config.js

  • yarn run-in-foo would show error as the nullishCoalescingOperator is not recognized, which means foo/babel.config.js is used without merging with the root babel.config.js

@phated
Copy link
Member

phated commented Dec 27, 2018

@ljqx I'm looking into this deeper and I'm wondering if we should use "upward-optional" instead (see https://babeljs.io/docs/en/options#rootmode)

@phated
Copy link
Member

phated commented Dec 27, 2018

It also seems like if you don't have a babel.config.js that an error will be thrown - I don't think I like that.

@phated
Copy link
Member

phated commented Dec 27, 2018

cc @hzoo - maybe you can give more insights.

@phated
Copy link
Member

phated commented Dec 29, 2018

@ljqx now that we have proper tests on this repo, I made the change you are suggesting and the tests are failing when users are using a .babelrc instead of babel.config.js so it doesn't look like we can use upward.

edit: upward-optional seems to allow the tests to pass.

@phated
Copy link
Member

phated commented Dec 29, 2018

I'd also like to add a test to the suite but I think that might collide with the .babelrc test.

@phated phated mentioned this issue Dec 29, 2018
@ljqx
Copy link
Author

ljqx commented Dec 29, 2018

@phated hmm, yeah, seems for people who don't have babel.config.js, upward-optional is more suitable.

And both upward & upward-optional would find the root folder only by babel.config.js. Other config files like .babelrc belongs to file relative configuration.

So seems no option equivalent to previous one in Babel 6 (upward-optional but fallback to topmost folder instead of cwd).

phated pushed a commit that referenced this issue Oct 16, 2019
…ward-optional mode (fixes #39, #41, #54) (#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR #41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
phated pushed a commit that referenced this issue Oct 16, 2019
…ward-optional mode (fixes #39, #41, #54) (#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR #41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
HRKings pushed a commit to HRKings/interpret that referenced this issue Jan 15, 2022
…ward-optional mode (fixes gulpjs#39, gulpjs#41, gulpjs#54) (gulpjs#58)

* babel must now ignore files not ending by babel.js (inspired from @cspotcode branch, PR gulpjs#41 extending ignoring to everywhere babel register is called)
* @babel/register now uses upward-optional mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants