ankitrg
commented
7 years ago Still facing this after https://github.com/rails/webpacker/pull/291 I think I have the latest version from the master!
Closed ankitsinghaniyaz closed 7 years ago
ankitrg
commented
7 years ago Still facing this after https://github.com/rails/webpacker/pull/291 I think I have the latest version from the master!
gauravtiwari
commented
7 years ago Have you re-run webpacker:install and replaced outdated configs?
ankitrg
commented
7 years ago @gauravtiwari as I mentioned I ran both the command again and I overwrote all the files. Any idea why this might be happening? For now is there a way to disable uglify, so that at least I can push updates to my production app.
gauravtiwari
commented
7 years ago Ahh strange! Sure just remove uglify from production.js for now. I will test again, it's seems like certain syntax isn't supported or perhaps requires a babel plugin 🙂
gauravtiwari
commented
7 years ago If possible could you please share your vue app code or equivalent example so I can test it out at my end.
ankitrg
commented
7 years ago Sure, I'll share my code.
RavenXce
commented
7 years ago That error occurs because uglifier-js doesn't support shorthand method syntax, i.e. myMethod() { .. }, (or ES6 in general).
You need to lock uglifier in package.json to "uglify-js": "git://github.com/mishoo/UglifyJS2#harmony", if you want to use that syntax.
See: https://github.com/lautis/uglifier#uglifier and https://github.com/mishoo/UglifyJS2/issues/448 for more details
Edit: It seems from that issue thread that the community is moving towards babel minify (babili). We can perhaps consider that as the default compressor instead.
gauravtiwari
commented
7 years ago @RavenXce That's correct, but since this is compiled through babel it shouldn't be a problem. I think it needs required babel plugin to compile that syntax - https://babeljs.io/docs/plugins/transform-function-bind/
RavenXce
commented
7 years ago @gauravtiwari I believe the issue is that the env preset does not convert this as modern browsers already support it. Even if we specifically convert this syntax, other issues may pop up later on.
So the core issue is that the compressor must still support at least what browsers are currently supporting at the end of the day. I believe uglifyjs may not be up to this task.
RavenXce
commented
7 years ago Sorry I'm on mobile so I can't edit my above comment atm but I just wanted to add that I ran into this particular issue too, and the correct plugin to use is: https://babeljs.io/docs/plugins/transform-es2015-shorthand-properties/.
Edit: This babel option is also relevant to this issue, and gives the same alternative recommendations - https://github.com/babel/babel-preset-env#targetsuglify
gauravtiwari
commented
7 years ago Is this still an issue for you @ankitrg? @RavenXce So, it's just matter of adding correct plugin or preset right?
mlynarczyk
commented
7 years ago I know that not everyone will consider this a proper solution but:
yarn add babel-preset-es2015
and in .babelrc
{
"presets": ["es2015"]
}Solves the issue described here.
gauravtiwari
commented
7 years ago Just use this for now, will get this updated soon 👍
{
"presets": [
["env", {
"modules": false,
"targets": {
"node": "current"
},
"useBuiltIns": true
}],
"react"
]
}@gauravtiwari depending on what we want, I think there are three options we can look at to make things less confusing for users. Right now, almost everyone will run into this issue as the chosen default babel preset doesn't match what uglify supports.
{
"presets": [
["env", {
"modules": false,
"targets": {
"uglify": true,
}
]
}This should work, but will basically cause babel to always transpile to ES5, which also means features which browsers already support will still be polyfilled regardless of what the user wants to target. May not be ideal as a default because of that.
# in package.json:
"uglify-js": "git://github.com/mishoo/UglifyJS2#harmony"This allows uglify to support most ES6 syntax. It is what I'm currently doing at the moment. However, it may not be ideal as a default too, as there are no actual releases that support ES6 (have to use git url to target the branch in package.json).
I have not tried this, but seems to be the current go-to for minifying JS? Is meant to support ES6 as well.
gauravtiwari
commented
7 years ago Alright, closing this for now 👍 Lets use what we have for now and revisit later once of the options above are usable.
flybayer
commented
7 years ago I'm using option 1 as suggested by @RavenXce, but I also had to install and use babel-polyfill
gauravtiwari
commented
7 years ago @flybayer Are you using useBuiltIns option too?
flybayer
commented
7 years ago @gauravtiwari, yes, I am.
gauravtiwari
commented
7 years ago And you still have to use babel-polyfill? Per documentation it feels like, even if you use uglify the built-in option should work just fine - https://github.com/babel/babel-preset-env#targetsuglify
flybayer
commented
7 years ago I know :/
Yes, I still have to use babel-polyfill
{
"presets": [
["env", {
"modules": false,
"targets": {
"node": "current",
"uglify": true
},
"useBuiltIns": true
}]
],
"plugins": [
"transform-object-rest-spread"
],
}Hmm strange.
gauravtiwari
commented
7 years ago I will file an issue on the preset repo 👍
RavenXce
commented
7 years ago @flybayer you need to use babel-polyfill because you are setting useBuiltIns. See: https://github.com/babel/babel-preset-env#usebuiltins
Don't think it has anything to do with this issue @gauravtiwari
gauravtiwari
commented
7 years ago @RavenXce Right, but that's what useBuiltIns option is for to replace any import 'babel-polyfill' statements.
This option enables a new plugin that replaces the statement import "babel-polyfill" or require("babel-polyfill") with individual requires for babel-polyfill based on environment.
The ugilify option states this, if I am reading it correctly:
To prevent these errors - specify the uglify option, which will enable all plugins and, as a result, fully compile your code to ES5. However, the useBuiltIns option will still work as before, and only include the polyfills that your target(s) need.
RavenXce
commented
7 years ago @gauravtiwari I think you're misinterpreting the instructions. It just means that useBuiltIns will replace the import line such that only required polyfills are imported instead of all polyfills, based on whatever env we are targeting in the options. It does not replace the plugin itself - it just allows it to be used in a configurable manner.
It is clarified later in the documentation:
In import "babel-polyfill"; Out (different based on environment) import "core-js/modules/es7.string.pad-start"; import "core-js/modules/es7.string.pad-end"; import "core-js/modules/web.timers"; import "core-js/modules/web.immediate"; import "core-js/modules/web.dom.iterable";
gauravtiwari
commented
7 years ago @RavenXce Oh yes, that's I agree that we need babel-polyfill installed.
gauravtiwari
commented
7 years ago It's just that we don't need to explicitly declare the imports right?
RavenXce
commented
7 years ago Users will still need to declare the babel-polyfill import at the top of their app.
Actually I'm not sure why useBuiltIns is being added? We shouldn't use babel-polyfill or add useBuiltIns as a default at all - they are only needed for those who want to use ES6 functions that has not landed on browsers such as String.padEnd(). Otherwise, it will only add to the file size of the compiled JS.
We should simply show how to enable them in the README under "advanced configuration"? I've made the PR which removes useBuiltIns instead - let me know if we still want the polyfill as default for some reason.
gauravtiwari
commented
7 years ago @RavenXce think async and await also needs babel-polyfill right?
RavenXce
commented
7 years ago Yup, that's correct.
I think I am missing something, I updated my gems and since then this is failing.
I also ran
Here is my trace: