Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create-react-app TypeScript @property decorator not creating accessors correctly #878

Closed
2 of 6 tasks
karlbennett opened this issue Jan 16, 2020 · 7 comments
Closed
2 of 6 tasks

Comments

@karlbennett
Copy link

karlbennett commented Jan 16, 2020

Description

When using the default TypeScript settings in create-react-app it seems that the @property decorator isn't creating the accessors correctly. Strangle running a delete (delete this.text) on the property fixes the accessors.

Workaround

To get the accessors working for the moment just run a delete on the property you created in the constructor.

@customElement("my-element")
export class MyElement extends LitElement {
    @property() text: string;

    constructor() {
        super();
        delete this["text"]; // This will fix the accessors.
        this.text = "Initial text.";
    }

    render(): TemplateResult {
        return html`<p>${this.text}</p>`;
    }
}

Demo

https://github.com/karlbennett/litelement-property-bug

Steps to Reproduce

Example:

  1. Create my-element
  2. Add a text property to my-element with the @property decorator.
  3. Use the my-element tag in some HTML.
  4. Select the my-element tag instance.
  5. Assign a new value to the text property (myElement.text = 'some new text.';).

Expected Results

A property update should trigger update events/methods (update, updated);

Actual Results

The events/methods are not triggered on a property update.

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 11
  • Safari 10
  • IE 11

Versions

  • lit-element: 2.2.1
@tpluscode
Copy link

Strangely running a delete (delete this.text) on the property fixes the accessors.

The accessor looks to be correctly added to the prototype but somehow it's being overridden again on the instance with a standard JS property. This is why deleting it allows the decorated accessors to kick-in

@justinfagnani
Copy link
Contributor

Is create-react-app using the useDefineForClassFields configuration option? That uses Object.defineProperty() on the instance for fields which will shadow the prototype accessors. We don't support that flag yet. See: microsoft/TypeScript#35081

@justinfagnani
Copy link
Contributor

Assuming this is a duplicate of #855 since we haven't heard otherwise.

@emilio-martinez
Copy link
Contributor

Hey @justinfagnani, just happened to stumble upon this issue while Googling, and thought I'd just add color while I'm here: create-react-app uses babel, so this issue likely refers to an issue with Babel (and maybe Babel's Typescript transform), and not Typescript itself.

That said, this issue could be in the neighborhood of #855 in terms of output (I haven't compared the two in detail), but it would relate to Babel, not Typescript.

@justinfagnani
Copy link
Contributor

Ok, thanks for the detail @emilio-martinez!

@owen26
Copy link

owen26 commented Jan 15, 2021

I finally got to the bottom of this issue so just want to share here in case other people encountered it.

Recently I need to integrate a web application written by lit-element into a react app powered by @nrwl/nx. Not so long before I realise all the button clicks in the app are not working anymore.

Then I created a minimum replication with create-react-app + lit-element. However, if I manually configure a webpack + babel + typescript + react project, it seems working fine.

Took me a while before I realised what's going on here. The issue is caused by legacy flag from '@babel/plugin-proposal-decorators decorator.

As you can see from the babel script that create-react-app uses:

https://github.com/facebook/create-react-app/blob/0f6fc2bc71d78f0dcae67f3f08ce98a42fc0a57c/packages/babel-preset-react-app/create.js#L203

It is set to legacy by default.

Also for Nrwl/nx based monorepo projects, it's the same situation:

https://github.com/nrwl/nx/blob/4193d451b0e6e472218defb291d0a015fb01952f/packages/web/babel.ts#L45

After overriding this setting from legacy to decoratorsBeforeExport, everything goes back to normal.

Someone also mentioned this issue here with storybook related stack, it seems all relevant.

storybookjs/storybook#12578

@tommck
Copy link

tommck commented May 1, 2022

@owen26 how, exactly, did you get this to work with @nrwl/nx ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants