Skip to content

Fix user config options not being applied when they are imported from an es6 module with babel6 #2376

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

Closed
wants to merge 1 commit into from

Conversation

LegNeato
Copy link

@LegNeato LegNeato commented Apr 22, 2016

This bug exists in webpack1 and webpack2. This patch is against master, let me know if you want me to backport. As people upgrade to babel 6 more and more are going to hit it...

Bug could manifest in two ways:

  • webpack1: invalid argument pathToArray error. This happens because the default for output.path is "" in webpack1
  • webpack2: compile not happening, webpack() returning super early but no errors thrown and no bundles created. This happens because the default for output.path is cwd(). Note: it works from the command line because the check was already there in the CLI parsing code, just not the node API.

This bug caused my working webpack config with babel5 to fail when upgrading to babel6. This was mainly due to my custom output.path being overwritten, so maybe other errors manifest differently than above. Also note the webpack1 error I got is hard to google...it usually is a symptom of people not putting in absolute paths to output.path...but in this case it is specified and webpack overwrites it as mentioned above.

This took me way too long to debug, mainly because there is a lot of indirection and the webpack Options have the concept of a "default" that isn't the same as an ES6 default of course! 🔎

@LegNeato
Copy link
Author

LegNeato commented May 3, 2016

👉 Friendly poke! Let me know if you need any changes.

sokra added a commit that referenced this pull request May 5, 2016
i. e. when passing a es6 module like #2376
@LegNeato
Copy link
Author

Any chance this can get merged? I keep having to patch it locally. Let me know if there is anything I need to do!

@codecov-io
Copy link

codecov-io commented May 25, 2016

Current coverage is 83.64%

Merging #2376 into master will decrease coverage by 7.50%

@@             master   #2376   diff @@
=======================================
  Files           231     229      -2   
  Lines          9088    8775    -313   
  Methods        1660       0   -1660   
  Branches       1808    1698    -110   
=======================================
- Hits           8283    7340    -943   
- Misses          805     887     +82   
- Partials          0     548    +548   

Powered by Codecov. Last updated by abf0cef...f0c5e6a

@LegNeato
Copy link
Author

🔔 Ping.

@LegNeato
Copy link
Author

@sokra anything I need to do here?

1 similar comment
@LegNeato
Copy link
Author

@sokra anything I need to do here?

@sokra
Copy link
Member

sokra commented Aug 17, 2016

I'll not merge it.

It not the job of the webpack node.js API to care for modules. It just expects a options object (not a module).

The CLI care for the module loading and it takes care of ES6 modules.

If using the node.js API you need to care for module loading yourself.

@sokra sokra closed this Aug 17, 2016
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 this pull request may close these issues.

3 participants