npm run build: fails in node_module on "class" or prototype defination

[jcalfee]

Is this a bug report?

Not sure

Can you also reproduce the problem with npm 4.x?

Yes

Which terms did you search for in User Guide?

deploy
configure

Environment

1. node -v: v8.2.1
2. npm -v: 5.3.0
3. yarn --version (if you use Yarn):
4. npm ls react-scripts (if you haven’t ejected): react-scripts@1.0.12

Then, specify:

1. Operating system: ubuntu 16.04
2. Browser and version (if relevant):

Steps to Reproduce

(Write your steps here:)

1. npm i eosjs-ecc
2. require('eosjs-ecc')
3. npm run build

Expected Behavior

success

Actual Behavior

npm run build

> react-scripts build

Creating an optimized production build...
Failed to compile.

Failed to minify the code from this file: 

 	../ecc/src/key_public.js:25 

Read more here: http://bit.ly/2tRViJ9

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! keygen@0.1.0 build: `react-scripts build`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the keygen@0.1.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/.npm/_logs/2017-09-01T17_29_14_675Z-debug.log

Line 25 is either "class" in or in my local testing the first nonoccurence of "PublicKey.prototype.xxx ="

Reproducible Demo

none yet

---

Prototype and classes are pretty standard I'm not sure why this would fail..

[Timer]

Read more here: http://bit.ly/2tRViJ9

npm run build fails to minify

You may occasionally find a package you depend on needs compiled or ships code for a non-browser environment.
This is considered poor practice in the ecosystem and does not have an escape hatch in Create React App.

To resolve this:

1. Open an issue on the dependency's issue tracker and ask that the package be published pre-compiled (retaining ES6 Modules).
2. Fork the package and publish a corrected version yourself.
3. If the dependency is small enough, copy it to your src/ folder and treat it as application code.

Looks like the package is intended to be used in Node, not on the web.
You can follow the instructions above or eject and make the modifications you need if you must use this on the web.

[jcalfee]

This is an opinion..

[Timer]

That may be so, so I'm sorry that Create React App doesn't work for your use case.
However, there is currently no escape hatch in Create React App -- this has been a long debated topic and we've decided to stick with it.

Feel free to eject if the above options do not work for you.

[Timer]

You can follow #2108 which would allow using class, but drops older browser support.

[gaearon]

We’ll eventually switch to Uglify v3 that allows it.

[jcalfee]

It is easy for me to avoid prototype and class in the future. It is just confusing, I don't really have the best javascript history knowledge yet. And it was a little strange to see prototype working the the browser but be unable to package it.

Thanks for the feedback.. Probably a good plan to upgrade eventually..

[jcalfee]

Uglify v3 would be much better .. Look like it can't handle default parameter values either.

[Timer]

Uglify v3 would be much better

The problem here is that you will ship code in production that will bomb out for users without browser support.
The current situation is convenient because it catches syntax bugs that would otherwise make it into a release which we say is syntactically valid for IE 9+.

[Timer]

When we switch to Uglify v3, we're going to need to also make sure syntax is parsed strict with the browsers you've specified to target, that way you don't accidentally ship bad syntax and break parsing on older browsers.

[jcalfee]

I understand.. We are developing blockchain apps that make use of crypto and we want very much for those old browsers to bomb it would be a security risk.. It would be nice of this were an option.

[Timer]

Ah, that's exactly what #892 will be! 😄 Thanks for the context.

For instance, you'll be able to target like:

  "browsers": {
    "development": [
      "last 1 chrome version"
    ],
    "production": [
      "last 4 versions",
      "not ie < 11"
    ]
  }

These wouldn't be runtime-enforced, however -- you'd need to do that yourself. Or maybe we could add an optional splash. 🤔

[jcalfee]

Your concern is about the build output code version. But this issue is about the version of the dependent modules going into the build.. These should be two different things.

How about run the babel es2015 conversion before Uglify v2? This is the code I used in my module to make it comparable with create-react-app. If create-react-app did this then many other modules would be spared from having to generate and deploy the generated copy to npm (which is a challenge to keep in sync).I think this is reasonable since old browsers do not run code directly from npm, they always go though a build process.

The trick is to create the more compatible generated code as late as possible.

node_modules/babel-cli/bin/babel.js src --out-dir lib

package.json

"babel": {
    "presets": [
      "es2015"
    ]
  }

[Timer]

@jcalfee ah, that's where our misunderstanding is!

We do not compile any code from node_modules, so it stays as-is. Even if we were to convert to Uglify v3, uses of class would leak into code targeting IE 9 since it could be parsed then, but then bomb out the browser.

We do not compile node_modules because it's incredibly slow and tends to break packages.