material-web: Components don't build properly with PolymerLabs/lit-element-build-rollup

Reproduce:

Clone https://github.com/PolymerLabs/lit-element-build-rollup
Update lit-element to latest (2.1.0)
Build and verify that build works (it does 😉)
Add a component e.g. npm install @material/mwc-button & import/add in my-element (The element version is 0.5.0)
Clean and build

Two things happen:

the build throws an error:

src/index.js → build/index.js...
(!) `this` has been rewritten to `undefined`
https://rollupjs.org/guide/en#error-this-is-undefined
node_modules/@material/mwc-button/mwc-button.js
1: var __decorate = this && this.__decorate || function (decorators, target, key, desc) {
                    ^
2:   var c = arguments.length,
3:       r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc,
...and 1 other occurrence

and 2. the app never renders - but throws an error in the console:

util.js:43 Uncaught TypeError: Cannot read property 'appendChild' of null
    at detectEdgePseudoVarBug (util.js:43)
    at supportsCssVariables (util.js:76)
    at ripple-directive.js:26

Using the rollup script from https://open-wc.org/building/building-rollup.html works with the mwc-elements.

Also, other elements done with lit-element work (with the PolymerLabs/lit-element-build-rollup script).

Theory: The TypeScript compiler creates extra ‘boilerplate’ code for properties that is not compatible with the simple rollup script.

‘captaincodeman’ (polymer slack) suggested adding this to the typescript config of the components (I didn’t have a chance to try yet):

"noEmitHelpers": true,
"importHelpers": true,

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 1
Comments: 15

Most upvoted comments

It seems like there might be a de facto standard of sorts where folks transpile the guts of a module but leave the import/export syntax in place. If I were transpile inert to ES5 but leave the import/export syntax would that negatively affect y’all?

It won’t break anything in MWC, but it will probably slightly increase bundle size (since transpiled classes are larger), and it makes the code a little less debuggable in the browser. I don’t think this module actually has any imports/exports, it’s all side-effects. (But if it did, shipping ES5 transpiled code but with module syntax seems pretty odd, because you don’t get the benefit of it working out-of-the-box in older browsers, but you also remove the possibility of serving smaller/faster code to modern browsers).

Sounds like this should have been a breaking change (totally my bad too, since we discussed it and I didn’t know about this kind of use case), but TBH I don’t think the fact that certain webpack configurations expect the “module” field to contain ES5 code is a compelling reason to shy away from publishing ES2015 to NPM. IMO if a user needs to support older browsers, they should have a build process that transpiles all their code, including dependencies, to ES5.

aomarks on Sep 12, 2019

I’ll re-open this until the wicg-inert issue is fixed and we bump the dependency here.

aomarks on Sep 4, 2019

@aomarks I filed a separate issue against the wicg-inert repo (see above) that appears to be a dependency for certain components. Does it make sense to reopen this issue until the dependency issue is resolved as you’ll likely need to update package.json for any affected components?

For example, mwc-drawer won’t function within a Rollup bundle until this is resolved.

https://github.com/material-components/material-components-web-components/blob/master/packages/drawer/package.json#L23

mi-ztroh on Sep 4, 2019

@aomarks When can we expect NPM component releases including this fix? Thanks!

Just released 0.8.0 with this change in it. Enjoy!

aomarks on Sep 4, 2019

(Copying my comment from #323 here)

We should set importHelpers: true and add a dependency on tslib so that the helpers like __decorate are imported from that module instead of inlined. This should also help code size and performance, since currently every element has its own duplicate of __decorate (and maybe some other helpers), not to mention most typescript users are also going to need this function too.

(This is arguably also a tsc bug, in that when emitting a module, this and var aren’t going to set globals, so not only is there a copy of each helper at every call site, there is no runtime de-duplication via globals.)

aomarks on Aug 14, 2019