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

loadPartialConfig fails to clear the overrides/test/include/exclude fields from its result #642

Closed
HsuTing opened this issue Jul 12, 2018 · 8 comments · Fixed by babel/babel#8315

Comments

@HsuTing
Copy link

HsuTing commented Jul 12, 2018

I'm submitting a bug report

Webpack Version:
3.10.0

Babel Core Version:
7.0.0-beta.53

Babel Loader Version:
8.0.0-beta.3

Please tell us about your environment:
OSX 10.13.5

Current behavior:
I give configFile as options like:

...
options: {
  configFile: '../../babel.config.js',
},
...

And this will not get any plugins or presets with babel.config.js.

Expected/desired behavior:

  • If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem along with a gist/jsbin of your webpack configuration.
    Just need to add babel.config.js to options.configFile.

  • What is the expected behavior?
    This should work with babel.config.js.

  • What is the motivation / use case for changing the behavior?
    I use babel.config.js in a mono repo. As a result, I want to babel.config.js can work, and I only need to write a config file to the root folder.

@HsuTing
Copy link
Author

HsuTing commented Jul 12, 2018

I think here cause this issue.
babel.loadPartialConfig will transform configFile to the real config, but the options of babel-core.transform need configFile.

Here is my patch files:

--- a/node_modules/babel-loader/lib/index.js
+++ b/node_modules/babel-loader/lib/index.js
@@ -79,12 +79,12 @@ function _loader() {
       throw new Error(`babel-loader ^8.0.0-beta.3 requires @babel/core@7.0.0-beta.41, but ` + `you appear to be using "${babel.version}". Either update your ` + `@babel/core version, or pin you babel-loader version to 8.0.0-beta.2`);
     }   
 
-    const config = babel.loadPartialConfig(programmaticOptions);
+    const config = programmaticOptions.configFile ? { options: programmaticOptions } : babel.loadPartialConfig(programmaticOptions);
 
     if (config) {
       let options = config.options;
 
-      if (overrides && overrides.config) {
+      if (overrides && overrides.config && !programmaticOptions.configFile) {
         options = yield overrides.config.call(this, config, {
           source,
           customOptions

This can work.
Hope this information is useful to you.

@loganfsmyth
Copy link
Member

but the options of babel-core.transform need configFile.

What makes you say that? With the current setup, config should have everything that would have been read from the config file, so reading it again would be extra work.

@loganfsmyth
Copy link
Member

Would you be able to provide an repository with an example of the config not working? I can look into it.

@HsuTing
Copy link
Author

HsuTing commented Jul 12, 2018

Maybe I misunderstand the function.
My repository is private, but I will try to make en example repository.
Thank you!

@HsuTing
Copy link
Author

HsuTing commented Jul 12, 2018

@loganfsmyth Sorry, I found this error is not made by configFile.
When I used overrides, overrides will make this error.
Here is my example repository.
Hope this will help you.

@loganfsmyth
Copy link
Member

Perfect, thanks. I see just what the problem is now.

@loganfsmyth loganfsmyth changed the title babel.config.js not work as configFile options loadPartialConfig fails to clear the overrides/test/include/exclude fields from its result Jul 12, 2018
@loganfsmyth
Copy link
Member

I've posted babel/babel#8315 which should address this once it is merged and published.

@HsuTing
Copy link
Author

HsuTing commented Jul 13, 2018

OK!
Thank you 😁!

loganfsmyth added a commit to babel/babel that referenced this issue Jul 13, 2018
| Q                        | A <!--(Can use an emoji 👍) -->
| ------------------------ | ---
| Fixed Issues?            | Fixes babel/babel-loader#642
| Patch: Bug Fix?          | Y
| Major: Breaking Change?  | N
| Minor: New Feature?      |
| Tests Added + Pass?      | Yes
| Documentation PR Link    | <!-- If only readme change, add `[skip ci]` to your commits -->
| Any Dependency Changes?  |
| License                  | MIT

Since these were getting left in, things that loaded the config, and then passed in back to Babel would get `test` and such _twice_, which could lead to either bad configuration merging, or no configuration at all if the patterns were relative to different directories, as was the case in babel/babel-loader#642.
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