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

Make decorators work with new decorator spec #156

Closed
justinfagnani opened this issue Aug 28, 2018 · 11 comments
Closed

Make decorators work with new decorator spec #156

justinfagnani opened this issue Aug 28, 2018 · 11 comments

Comments

@justinfagnani
Copy link
Contributor

Babel 7 has an implementation of the new spec. We should be able to make decorators detect how they're being called to work with either TypeScript or Babel 7.

@aadamsx
Copy link

aadamsx commented Aug 28, 2018

Yes, Babel7!!

@HitkoDev
Copy link

While we're at it, could we add TypeScript decorator metadata support, so we wouldn't have to define property types twice?

@justinfagnani
Copy link
Contributor Author

@HitkoDev unlikely for now, as Reflect.metadata isn't a maintained proposal, and the polyfill is quite large.

@HitkoDev
Copy link

HitkoDev commented Aug 29, 2018

I believe lit/lit#3064 would solve that problem, as it will remove decorators from the code hence no need for reflection polyfill to be included in the production build.

@abraham
Copy link
Contributor

abraham commented Aug 29, 2018

I've been working on a much smaller implementation of Reflect.metadata you could try for a polyfill.

https://github.com/abraham/reflection

@blikblum
Copy link

blikblum commented Jan 5, 2019

I evaluated the possible compilers setup to support LitElement property decorator.

The source code can be found here and the live examples here.

Basically the property decorator works with typescript but does not work with Babel 6 and Babel 7 (legacy). It can make work with Babel 7 (current spec) with a few modifications.

The reason to typescript work and babel 6 / babel 7 (legacy) not work, all based in same spec ,is that the former initializes the value using a direct assignment while babel uses a Object.defineProperty call.

@43081j
Copy link
Contributor

43081j commented Jan 7, 2019

Something worth noting that i seem to recall from last i investigated this, too, is that babel's output of a decorated property with a default value results in it producing a descriptor with value set (e.g. value: () => 'the default value').

This means the default value never triggers an initial render.

TypeScript produces a this.myProp = 'the default value' in the constructor which does trigger the usual lifecycle.

I may be wrong or this may have changed since then, but its what i ran into at the time and worth testing when someone tackles this.

@sorvell sorvell added this to the 1.0.0 milestone Jan 7, 2019
justinfagnani added a commit that referenced this issue Jan 9, 2019
* Moves @Property tests to decorators_test.ts
* Adds a Babel build step to compile decorators_test.ts to decorators-babel_test.js
* Updates decorators to detect if they’re called in legacy or modern decorator mode

Fixes #156
@justinfagnani
Copy link
Contributor Author

@43081j I worked around that in #422 by inducing a fake property with an initializer that sets the decorated property.

@littledan, you might be interested in the need to work around this. It turns out that an "initializer" as a class element isn't sufficient for us, because we still must return some element descriptor, so we still needed the fake property descriptor.

justinfagnani added a commit that referenced this issue Jan 9, 2019
* Moves @Property tests to decorators_test.ts
* Adds a Babel build step to compile decorators_test.ts to decorators-babel_test.js
* Updates decorators to detect if they’re called in legacy or modern decorator mode

Fixes #156
@littledan
Copy link

littledan commented Jan 9, 2019

Yeah, we are adding a separate standalone initializer which doesn't bother defining a property, since that's a pretty common need. Took us some time to bikeshedding the name (maybe it will be {kind: "hook", start() {}}), but it got consensus at the last TC39 meeting (slides). Would that solve the problem for you?

kevinpschaaf pushed a commit that referenced this issue Jan 10, 2019
* Support new decorators proposal as implemented in Babel 7

* Moves @Property tests to decorators_test.ts
* Adds a Babel build step to compile decorators_test.ts to decorators-babel_test.js
* Updates decorators to detect if they’re called in legacy or modern decorator mode

Fixes #156

* Address review feedback.

* Lint fixes.

* Separate babel tests and feature detect browser support for babel

* [no ci] Update changeling
@trusktr
Copy link

trusktr commented Sep 9, 2020

related: #234 (comment)

@bschlenk
Copy link
Contributor

@HitkoDev I wrote a babel plugin that can automatically set lit-element's property type based on the typescript type info - no need for runtime reflection. https://github.com/bschlenk/babel-plugin-lit-property-types-from-ts

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

10 participants