lit / lit-element

LEGACY REPO. This repository is for maintenance of the legacy LitElement library. The LitElement base class is now part of the Lit library, which is developed in the lit monorepo.
https://lit-element.polymer-project.org
BSD 3-Clause "New" or "Revised" License
4.49k stars 318 forks source link

`@property` / `static get properties` break in certain webpack/babel configurations #1135

Closed matthias-ccri closed 3 years ago

matthias-ccri commented 3 years ago

This is certainly a weird issue. I spent most of yesterday and today debugging this and narrowing it down in a repro. Apologies for the complexity, but it involves babel and webpack and storybook. I have not been able to reduce it further, but I would appreciate help in doing so, or perhaps this is enough.

In essence, it looks like code transpilation can disable @property and static get properties and make them not function. I'm making this issue to perhaps help people who might also run into this, and myself of course. It would be worth documenting as a caveat whatever the root cause turns out to be.

Description

I'm seeing an issue where 1) a property declared with the @property decorator fails to work, compared to to static get properties. 2) in a different babel configuration it's the reverse: static get properties fails to work, but @property works.

By "work" I mean trigger component re-render when the property changes.

Live Demo

https://codesandbox.io/s/loving-violet-rfk7c?file=/stories/Example.stories.ts

This is a barebones @storybook/html setup. Storybook uses its own webpack config for transpiling the story code. I was unable to repro with plain webpack (attempt). So maybe this is caused by a bad combination of babel plugins or options.

Steps to Reproduce

  1. View the storybook page. It shows elements toggling between two states: on and off.

Expected Results

All three elements It should cycle between on and off.

Actual Results

The "B" component does not update.

In .storybook/main.js you can uncomment a babel configuration that follows the recommendations in Build for production. However, once applied, component "A" now breaks and "B" starts working. Happy Wednesday.

Browsers Affected

Versions

Note: the repro uses @storybook/html instead of @storybook/web-components because I had dependency issues with the latter. Maybe I should give it another shot? Anyway, this issue seems worth reporting regardless.

matthias-ccri commented 3 years ago

Related: https://github.com/Polymer/lit-element/issues/156 https://github.com/Polymer/lit-element/issues/205 https://github.com/Polymer/lit-element/issues/234

kevinpschaaf commented 3 years ago

Just to confirm... you're authoring in TS, but are you using TS's decorator transform, or relying on babel for that?

The root cause looks to be a flavor of https://github.com/Polymer/lit-element/issues/1030 (and an issue with transforming class field initializers to the defineProperty semantics of class fields).

Looking at the compiled source, we see the working one uses assignment, the failing one uses defineProperty (which shadows the prototype accessor used for triggering updates by defining a property value on the instance):

  function ThingA() {
   ...
    // GOOD ======================
    _this3.isOn = void 0;
    ...
  }
var ThingB = (_dec3 = Object(lit_element__WEBPACK_IMPORTED_MODULE_2__["customElement"])('thing-b'), _dec4 = Object(lit_element__WEBPACK_IMPORTED_MODULE_2__["property"])({
  type: Boolean
}), _dec3(_class5 = (_class6 = (_temp3 = /*#__PURE__*/function (_LitElement3) {
   ...
    // BAD ======================
    _initializerDefineProperty(_this4, "isOn", _descriptor, _assertThisInitialized(_this4));
   ...
  }

Compiling with TS and ensuring useDefineForClassFields: false can work around this. Not sure if there is a babel equivalent.

matthias-ccri commented 3 years ago

Yes, I'm running the Ts through babel-loader via storybook's webpack config.

It sounds like switching to ts-loader is a viable workaround.

And when publishing as ES2017, it looks like typescript transforms away the decorators into a more compatible syntax.

e111077 commented 3 years ago

I think this is causing issues with the default setup on codesandbox.io repro with @lit-labs/react as reported by a user on Slack.

https://codesandbox.io/s/cool-voice-mnwxv?file=/src/App.tsx

I also think this is the cause of another issue reported by a community member on slack. I was able to make a glitch repo of their project. The issue is that @state is not updating on tun-player on this one:

https://glitch.com/edit/#!/enchanted-wax-drum?path=src%2Fcomponents%2Ftun-player.ts%3A25%3A0

users' solution on the second one was to add the following (which throws a warning at build for having both setPublicClassFields: true and loose: true, but having either or will not fix it.

image

Regardless, this leads to issues with projects where you cannot access the babel config like the codesandbox example

EDIT: I have a minimal repro

https://glitch.com/edit/#!/auspicious-handsomely-scabiosa?path=babel.config.json%3A8%3A63