autobind-decorator: Uglify errors in react scripts

@stevemao I read through it, but I didn’t understand how to take action. I can’t stop using react-DnD, and the older version of autobind-decorator works well for my current needs.

@stevemao, For now, we need to look at a workaround. The default create-react-app / react scripts does not support this code. And most devs use the create-react-app. Hence a lot of people who have autobind-decorator dependency will run into this error soon. I personally wasted a day before I tracked this issue to this library.

My recommendation is to create a 3.x version with ES6 features and maintain a 2.x release for ES5 until react start supporting ES6 by default. What do you think?

Related Bug: https://github.com/react-dnd/react-dnd/issues/1152

@shouvik44 did you read the blog I linked?

Related Bug: react-dnd/react-dnd#1152

I couldn’t build my app built on Create-React-App using React DnD. Thanks @harinair for pointing to the related thread. If others are experiencing this, the workaround with react dnd is to pin autobind-decorator to version 2.1.0. Since then, I’m able to build just fine.

Definitely should have been a version bump as this is definitely a breaking change.

If we pointed main to a ES6 script then it would have been a breaking change.

Thanks @jharris4 . I personally periodically improve my build script and look up create-react-app as a good reference. They have removed deprecated uglify-js and they also compile node_modules (but only from standard javascript).

lol, except you can’t use minification anymore with webpack without transpiling node modules first.

The right way to fix it is to use separate lib/es outputs like most other libraries do.

Happy to offer pointers if you’re open to it. It’s just a matter of forking your babel config to change the modules parameter to either preset-es2015 or preset-env…

A good example is here: https://github.com/react-bootstrap/react-bootstrap/blob/master/.babelrc.js

In any case, we only added a module entry to the package.json for modern browsers and tree shaking. So it’s a new feature. The old main entry is still there any the files are not changed. If you want the old file you can still use the main entry so this is not a breaking change.

I have never seen a single other package that exports raw es6 source via the modules field

I have seen many packages exporting modern javascript via the main field. This is also the reason why create-react-app compiles node_modules.

In any case, speaking of “modern javascript” the new “class properties” proposal & babel-plugin let you just replace:

That is already mentioned in the README and I think it’s a good alternative https://github.com/andreypopp/autobind-decorator#alternative

I guess a year and a half is a long time in JS land. lol For what it’s worth, Dan Abromov (author of create react app) also said the same thing (a year ago): https://github.com/webpack/webpack/issues/1979#issuecomment-339738809

If you want to deviate from the original intention of the modules field then that’s your prerogative, but you’re going against the grain (I have never seen a single other package that exports raw es6 source via the modules field) and you’re forcing people to compile all of their node_modules, which is certainly not standard.

I guess I’ll leave it here and let other users of this package chime in if they feel the need to.

In any case, speaking of “modern javascript” the new “class properties” proposal & babel-plugin let you just replace:

class Foo {
  @autobind
  aMethod() { }
}

with:

class Foo {
  aMethod = () => { }
}

This is a simpler alternative for our projects than having to compile all source in node_modules to be able to use new versions of this package…

modules should point to ES5 code with ONLY es6 modules.

I don’t necessarily agree. That article was published a long time ago when es6 was new (the future javascript). Some people only want to target modern browsers (with full support of modern JavaScript), are they forced to run es5 code only?

Have you tried bundling the latest of autobind-decorator with either rollup or webpack?

I’m using webpack and I even target ie11 because of the business requirements. I always use Babel to transpile node modules from modern to targeted javascript env (this is what create react app is doing).

Sent from my iPhone

On 7 Nov 2018, at 8:29 am, Jon Harris notifications@github.com wrote:

Who says that? Module entry is supposed to point to scripts with Es modules. Tools usually use this entry for tree shaking.

Did you read the link I posted? Basically the authors of webpack & rollup ALL say that And since they are the ones who added it to package.json and implemented tree shaking they’re probably the best authority on the subject.

On the other hand, you are using a deprecated minifier and wrong webpack config.

I’m using the latest version of webpack and the latest minifier that it supports. I maintain several projects that use webpack, so I hardly think I’m using a wrong webpack config. It worked fine until the version of autobind-decorator that broke the rules.

If you need to support old environment, you can use Babel env to transpile modern JavaScript to old version.

That is not how the spec for modules in package.json modules works. If you don’t want to believe that then I guess this package has hit a dead end and people will need to find alternatives if they want to use autobind decorators with modern tools like webpack or rollup.

Have you tried bundling the latest of autobind-decorator with either rollup or webpack? I suggest you try…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Who says that? Module entry is supposed to point to scripts with Es modules. Tools usually use this entry for tree shaking.

Did you read the link I posted? Basically the authors of webpack & rollup ALL say that modules should point to ES5 code with ONLY es6 modules. And since they are the ones who added it to package.json and implemented tree shaking they’re probably the best authority on the subject.

On the other hand, you are using a deprecated minifier and wrong webpack config.

I’m using the latest version of webpack and the latest minifier that it supports. I maintain several projects that use webpack, so I hardly think I’m using a wrong webpack config. It worked fine until the version of autobind-decorator that broke the rules.

If you need to support old environment, you can use Babel env to transpile modern JavaScript to old version.

That is not how the spec for modules in package.json modules works. If you don’t want to believe that then I guess people will need to find alternatives if they want to use autobind decorators with modern tools like webpack or rollup.

Have you tried bundling the latest of autobind-decorator with either rollup or webpack? I suggest you try…

You can read more about it here: https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

let is not future JavaScript. It became standard 3 years ago. If I published typescript, stage 3 features/syntax or flow then its a bug and should be transpiled. If you need to support old environment, you can use Babel env to transpile modern JavaScript to old version.

Sent from my iPhone

On 7 Nov 2018, at 7:05 am, Jon Harris notifications@github.com wrote:

@stevemao could you please remove modules from package.json?

The code pointed to by modules needs to be ES5 code but with ES6 modules (import/export).

The problem here is that src had ES6 code (including let etc).

You can read more about it here: https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

It’s definitely a bug related to the contents of the package.json now having “module”

How is adding modern JavaScript to module is a bug? We still have main entry which point to the same old JavaScript so its not a breaking change. On the other hand, you are using a deprecated minifier and wrong webpack config.

The code pointed to by modules needs to be ES5 code but with ES6 modules (import/export).

Who says that? Module entry is supposed to point to scripts with Es modules. Tools usually use this entry for tree shaking.

Sent from my iPhone

On 7 Nov 2018, at 7:05 am, Jon Harris notifications@github.com wrote:

@stevemao could you please remove modules from package.json?

The problem here is that src had ES6 code (including let etc).

You can read more about it here: https://github.com/rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

I just ran into the exact same issue.

It’s definitely a bug related to the contents of the package.json now having "module": "src/index.js", as added in this commit:

https://github.com/andreypopp/autobind-decorator/commit/9edeabf87c29d66e6d3d7ebffd715c05be119470#diff-b9cfc7f2cdf78a7f4b91a753d10865a2