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

Mixed ES6 and CJS causes issues in some enviroments #8

Closed
Hypercubed opened this issue Jul 2, 2016 · 8 comments
Closed

Mixed ES6 and CJS causes issues in some enviroments #8

Hypercubed opened this issue Jul 2, 2016 · 8 comments

Comments

@Hypercubed
Copy link

Thank you for ml-pca, we are using it in a browser project and so far looks great. One issue we are having, however, is that the JS files in ml-pca and ml-matrix use ES6 features within a CJS file. When using JSPM/SystemJS in the browser this means that the files are detected as CJS and not transpiled.

Here is a relevant SystemJS issue: systemjs/systemjs#811

One your end I can see three approaches:

  1. Use babel to transpile on publish.
  2. Use ESM import/export syntax (this may force you to do PCA crash if the data is only one element #1 also).
  3. Ignore it and wait for it to be fixed by JSPM/SystemJS (maybe next version).
@Hypercubed
Copy link
Author

Hypercubed commented Jul 4, 2016

I guess # 1 is out. It appears it is not possible to extend native classes using Babel.

@Hypercubed
Copy link
Author

Hypercubed commented Jul 5, 2016

I"ve looked in detail at this issue. There are several things at play:

  1. Because the modules are CJS, JSPM will not mark them for transpiling.
  2. Because they are not transpiled, ES6 features are left untouched when bundling. This prevents minification because UglifyJS does not support ES6.
  3. Forcing transpiling does not work because ml-matrix uses ES6 class syntax to extend Array. This is not something possible to support using a transpiler.

All this to say when using mljs tools in the browser minification (a common build step for browsers) is not possible as far as I can tell. I don't see a solution at the moment other than "don't minify". You might want to add this as a note somewhere.

Feel free to close this issue.

@maasencioh
Copy link
Member

Hello @Hypercubed, thanks for all the research and for use our packages, we'll add this request and as soon as we have any solution we'll notify this to you. Thanks again!

@targos
Copy link
Member

targos commented Jul 7, 2016

Thanks for the detailed description of your problem.
It indeed impossible to transpile ml-matrix because of it's use of extends Array. This limitation expands to all the projects that have it as a dependency (almost all mljs projects now...). I will add a note in the README of ml and ml-matrix about that.
Unfortunately, I haven't found a way to minify without transpiling.

UglifyJS is actively working on a version that can handle ES6 here. I will look into it and see what can be done with it.

@targos
Copy link
Member

targos commented Jul 29, 2016

@Hypercubed there were a few bug with the harmony branch of UglifyJS preventing the minification of ml-matrix. I believe they are now fixed.
If you can control what version of UglifyJS is used in your build phase, you can npm i github:mishoo/UglifyJS2#harmony --save-dev to use this branch.

@Hypercubed
Copy link
Author

I believe this is working. However, in my app UglifyJS2#harmony is now giving another error that I think is unrelated to ml-matrix. I'll try to isolate and submit an issue to UglifyJS2.

@Hypercubed
Copy link
Author

Indeed, the issue I am now having is unrelated to ml-matrix: mishoo/UglifyJS#1228

@Hypercubed
Copy link
Author

I managed to get ml-matrix@1.1.5 to transpile using babel-plugin-transform-builtin-extend. I had no luck with 2.0.0. I think the abstractMatrix abstraction is too much for the plugin.

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

No branches or pull requests

3 participants