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

lit decorators doesnt work with babel legacy mode decorators #205

Closed
mzeiher opened this issue Sep 18, 2018 · 16 comments
Closed

lit decorators doesnt work with babel legacy mode decorators #205

mzeiher opened this issue Sep 18, 2018 · 16 comments
Assignees
Milestone

Comments

@mzeiher
Copy link

mzeiher commented Sep 18, 2018

babel decorators (in legacy mode) expect the property descriptor to be returned rather than be defined within the decorator, TS also supports this feature.

Steps to Reproduce

checkout https://github.com/mzeiher/lit-element-test

npm install
npm run build
open index.html

Expected Results

both elements are updated (one decorated with TS the other with babel decorators in legacy mode)

Actual Results

only the TS version works

Browsers Affected

  • [ x] Chrome
  • [ x] Firefox
  • [ x] Edge
  • [ x] Safari 11
  • [ x] Safari 10
  • [ x] IE 11

Versions

  • lit-element: v0.6.1
  • webcomponents: v2.1.3
@mzeiher
Copy link
Author

mzeiher commented Sep 18, 2018

i prepared a pull request to support the babel decorators

@aadamsx
Copy link

aadamsx commented Sep 30, 2018

Related #174

Decorators don't work in Babel7, I think they work in Babel6

@aadamsx
Copy link

aadamsx commented Sep 30, 2018

i prepared a pull request to support the babel decorators

Do you have a link to the PR?

@blikblum
Copy link

#206

@mbrowne
Copy link

mbrowne commented Oct 7, 2018

I started investigating how this could work in Babel 7 with the latest decorators proposal. I'll post back here if I have time to work on it more, but this inspired me to make this comment on the proposal repo which might be of interest:
tc39/proposal-decorators#85 (comment)

@mbrowne
Copy link

mbrowne commented Oct 8, 2018

I figured out a working implementation of the @property decorator for Babel 7 (and latest decorators proposal, i.e. non-legacy mode):

export function property(options) {
    return elementDescriptor => {
        const name = elementDescriptor.key
        // key generation code copied from https://github.com/Polymer/lit-element/blob/master/src/lib/updating-element.ts
        const key = typeof name === 'symbol' ? Symbol() : `__${name}`
        return {
            // We are creating an own property and using the original initializer, but changing the key,
            // so foo becomes __foo. The getter and setter methods are created by createProperty().
            ...elementDescriptor,
            key,
            finisher(clazz) {
                clazz.createProperty(name, options)
            }
        }
    }
}

@mzeiher
Copy link
Author

mzeiher commented Nov 18, 2018

@mbrowne I had the same problem with my library, I wrote all my decorators according to the stage2 API and for legacy and babel compatibility I created a small helper to use the stage2 decorators also in legacy mode (https://github.com/mzeiher/ce-decorators/blob/master/src/stage2/stage2decorators.ts) the decorators probe if a stage2 or legacy is expected and apply either the native s2, or transform the request to a s2 compatible one (don't know if it works on all edge cases, it works for me at the moment with TS decorators (also patched Reflect.decorate to let TS apply multiple decorators (https://github.com/mzeiher/ce-decorators/blob/master/src/reflect.ts)) Babel legacy and Babel-stage2. maybe you can include this logic into lit-element? Or create maybe I can publish the helper as a separate library...

@ernsheong
Copy link
Contributor

ernsheong commented Nov 25, 2018

Is this also why I'm currently seeing (in latest release) that my use of @property decorators does not trigger render?

@thepassle
Copy link

@ernsheong Yep

@sorvell
Copy link
Member

sorvell commented Dec 10, 2018

Dup of #156

@justinfagnani
Copy link
Contributor

Re-opening as we've had a couple more requests. I'm not sure when we can get to this though, so contributions welcome.

@justinfagnani justinfagnani reopened this Mar 12, 2020
@blikblum
Copy link

From #921

I think we can make it work by checking the arguments to the decorators more carefully.

Sometime ago, i did some tests with possible decorator setups (babel6, babel7/legacy babel7/spec, typescript): https://github.com/blikblum/lit-element-decorator-tests

You can look at compiled files like https://github.com/blikblum/lit-element-decorator-tests/blob/master/babel7/dist/main-legacy.js#L2258

At the time i come to the conclusion that is not possible to use property decorator with babel/legacy and that typescript was working by accident (it is/was setting the property instead of defining it)

@omarbelkhodja
Copy link

How to reproduce with a project on github pwa-webpack-starter-kit. If you run the project using npm install && npm run start everything will work OK. If you change the babel configuration from
['@babel/proposal-decorators', { decoratorsBeforeExport: true, }],
to
['@babel/proposal-decorators', { legacy: true, }],
The elements won't be rendered.

@smart
Copy link

smart commented Jan 1, 2021

Struggling with this in a project that uses NX where I can't override { decoratorsBeforeExport: true } in the babel config built into their build system. I've opened a ticket but if there are other paths to resolution I'd appreciate any help, I was stumped on why the property changes weren't updating the updating the view for a long time.

@bschlenk
Copy link
Contributor

Looks like TC39 has come up with yet another decorators proposal, and recommends sticking with legacy decorators for now. The new proposal seems less powerful than either of the other two. Hopefully it will still support everything lit is using decorators for.

@sorvell
Copy link
Member

sorvell commented Jul 14, 2022

Closing due to age. We're waiting for spec progress before making additional changes to decorators support.

@sorvell sorvell closed this as completed Jul 14, 2022
@sorvell sorvell moved this to ✅ Done in Lit Project Board Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

14 participants