search

canjs / can-observable-mixin

Define properties on JavaScript classes
https://canjs.com
MIT License
2 stars 1 forks source link

Causes Webpack builds that use ES Modules and the master "can" package to crash IE 11 #125

Closed rjgotten closed 5 years ago

rjgotten commented 5 years ago

Currently the ES modules builds for CanJS always re-exports everything including can-define-mixin / can-observable-mixin and uses tree-shaking to remove unused parts in production.

Because of this, can-define-object and other dependents that use mixin-proxy will be loaded in development builds. This triggers an uncircumventable crash in Internet Explorer 11, because of an unguarded use of Proxy:

https://github.com/canjs/can-observable-mixin/blob/289f46c08aa844ba84d3fc5df1c32d4684b93f7b/src/mixin-proxy.js#L73

rjgotten commented 5 years ago

Added a pull request which elegantly fixes this problem,

It's based on the can-define-mixin-legacy branch though, because that's the package my employer is currently actually using.

We're currently using npm pack and a local .tgz installed into our projects to have this patched and get things working in IE 11. I just took the extra effort to also turn it into a pull request.

The work was done anyway and I figured you might appreciate someone sharing back. :smile:

phillipskevin commented 5 years ago

Thanks for this @rjgotten. Can you share how you have webpack set up? It's interesting that it doesn't do tree-shaking in development, so I want to make sure we cover this in our test suite.

justinbmeyer commented 5 years ago

I think @matthewp was aware of this ...

rjgotten commented 5 years ago

Can you share how you have webpack set up? It's interesting that it doesn't do tree-shaking in development, so I want to make sure we cover this in our test suite.

It's actually the default Webpack behavior; nothing custom involved. Webpack relies on the terser minifier to analyze unused exports and perform dead code removal on them. But that only works if the minifier is applied. Which is only the case when you use mode:"production".

See also: https://webpack.js.org/guides/tree-shaking/#clarifying-tree-shaking-and-sideeffects

rjgotten commented 5 years ago

@phillipskevin Will the changes from #126 at one point also be brought forward to the current 0.4.x can-observable-mixin? Just asking incase I rev my own projects to a newer CanJS version, so I know what I'll be getting into. :smile:

phillipskevin commented 5 years ago

@rjgotten yes, they will. If you want to put in a PR that would be great 😄. Or I'll take care of it when I get a few free minutes.

phillipskevin commented 5 years ago

This change has been rolled into all the different versions now. Thanks again @rjgotten.