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

Remove module field in production webpack config #2485

Closed
wants to merge 1 commit into from

Conversation

tlvince
Copy link

@tlvince tlvince commented Jun 6, 2017

This crops up in the form of uglify errors during a production build, e.g. #2236, #2433, #2475. The suggestion there is to transpile deps down to ES5 and/or remove module. This is correct with respect to main, however module is there specifically to indicate ES module support, which uglify doesn't support. Therefore, ask webpack not to resolve it.

You can test this by depending on a package that ships with multiple targets i.e. main pointing to a transpiled/common JS target and module targeting ES6+, e.g. tlvince-reduced-test-case-cra-module:

src/index.js

import test from 'tlvince-reduced-test-case-cra-module' 
test()

Before:

> react-scripts build

Creating an optimized production build...

Failed to compile.

static/js/main.1b21038d.js from UglifyJs
Unexpected token: punc (}) [./~/tlvince-reduced-test-case-cra-module/dist/tlvince-reduced-test-case-cra-module.es.js:1,14][static/js/main.1b21038d.js:79667,19]

After:

> react-scripts build

Creating an optimized production build...
Compiled successfully.

This crops up in the form of uglify errors during a production build, e.g. facebook#2236, facebook#2433, facebook#2475. The suggestion there is to transpile deps down to ES5 and/or remove `module`. However, `module` is there specifically to indicate ES module support, which uglify doesn't support, so ask webpack not to resolve it.
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@Timer
Copy link
Contributor

Timer commented Jun 6, 2017

We encourage the use of module as it should contain code which is just that -- modules. Code is supposed to be compiled down to the target ES version (in any package that supports a browser environment, ES5 or older).
Those issues you referred to are unfortunately caused by improper package publishing.

Packages shipping code which requires compilation should be using the esnext field, not the module field.

Please refer to the module field specification. Also see the lengthy discussion in webpack/webpack#1979.

@Timer
Copy link
Contributor

Timer commented Jun 6, 2017

While we greatly appreciate your contribution, this is not something we will be merging.
I look forward to contributions from you in the future!

@Timer Timer closed this Jun 6, 2017
@Timer
Copy link
Contributor

Timer commented Jun 6, 2017

For clarification, the issue isn't with modules; it's because the packages contain ES6 function syntax/object syntax (et al.), e.g. () => {} and:

let a = 5;
let b = { a };

@tlvince
Copy link
Author

tlvince commented Jun 6, 2017

Thanks! That and rollup wiki's pkg.module cleared it up for me.

@tlvince tlvince deleted the patch-1 branch June 6, 2017 22:36
@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants