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

Webpack - compile JS even from imported npm dependencies #3504

Closed
himdel opened this issue Mar 1, 2018 · 2 comments
Closed

Webpack - compile JS even from imported npm dependencies #3504

himdel opened this issue Mar 1, 2018 · 2 comments

Comments

@himdel
Copy link
Contributor

himdel commented Mar 1, 2018

Right now, any JS code in our code base gets compiled to es5, except for apparenly anything from npm.

Quick way to test:

--- a/app/javascript/packs/application-common.js
+++ b/app/javascript/packs/application-common.js
@@ -1,3 +1,6 @@
+console.log('XXXXXXXXXXXXXXXXXXXXXXXXXXXXX');
+require('proxy-polyfill');
+
 // This file is automatically compiled by Webpack, along with any other files
 // present in this directory. You're encouraged to place your actual application logic in
 // a relevant structure within app/javascript and only use these pack files to reference

and run (in ui-classic)

$ yarn add proxy-polyfill
$ bin/webpack
$ grep -Hn "const unsafeHandler" ../manageiq/public/packs/manageiq-ui-classic/application-common.js
../manageiq/public/packs/manageiq-ui-classic/application-common.js:72501:    const unsafeHandler = handler;

As long as const doesn't get changed to var, we can't really use npm dependencies written is ES6.

This also affects travis because spec:javascript will silently ignore any files with const inside:

Enabling show_console_log: true in spec/javascripts/support/jasmine.yml and running be rake spec:javascript | grep XXXXX ... as long as the require is there (and this is not fixed), no XXXXX will get output anywhere, changing the require to proxy-polyfill/proxy.min.js (which is already es5) will cause XXXX to appear.

Cc @karelhala @vojtechszocs
(But I'd like to take this one to learn a bit more about webpack, maybe update to webpack 4 in the process.)

@karelhala
Copy link
Contributor

I have solved this issue before, sadly don't remeber in which project and how 😞

But just by googling it looks like we have to add certain packages as non-exluded into build babel/babel-loader#171 (comment) (that's not how I solved it before, don't remeber really how, but definetely not like this 😄 ). I don't know if eslint will complain about this...

@himdel
Copy link
Contributor Author

himdel commented Mar 1, 2018

Interesting thread .. I wonder if I'm missing something but why do people even want to not compile something by default?

I ended up solving this by removing that exclude completely, I just can't imagine any case where compiling the dependencies would be the wrong thing to do.

(In fact, I was very surprised to find out that no other node modules in dependencies (as opposed to devDependencies) actually use const now, I would think that it is time for es2016+ js libraries now that webpack, etc. is being used ..even by us now :).)

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

2 participants