Documented useDefineForClassFields

[KeithHenry]

useDefineForClassFields is incompatible with experimentalDecorators, see lit/lit#1985

There are two solutions, both should be covered by Lit's documentation:

• Use useDefineForClassFields: false to force this new feature off.
• Use declare to tell TS not to emit inline class fields that block the decorator define function.

[aomarks]

Thank you for the PR! We've actually just landed a bunch of improvements to the docs around this issue in #516 which you can see live at https://lit.dev/docs/components/properties/#avoiding-issues-with-class-fields.

Closing because I think this supersedes the need for your improvement, but please let us know if you find something in this area is still missing or confusing.

[KeithHenry]

@aomarks

Thank you for the PR! We've actually just landed a bunch of improvements to the docs around this issue in #516 which you can see live at https://lit.dev/docs/components/properties/#avoiding-issues-with-class-fields.

Thanks and that's a lot more in depth, but it doesn't mention the declare option at all, and I think that might be a better option in new code than the compiler flag, as it will remain compatible with future TS changes (which seems unlikely for the flag).

[arthurevans]

Hi @KeithHenry. The engineer who wrote that section left the declare out intentionally. My understanding is that adding declare prevents you from initializing the property. For example:

@property()
declare name = 'No one';

Gives an error. Our preferred style is to initialize the field where we first declare it, and adding the declare keyword would break our sample code that uses this style. Hope that explains the thought process.

[KeithHenry]

@arthurevans @aomarks I'm aware of that, it should be in the documentation too. Yes, declare prevents inline initialisation, but useDefineForClassFields: false affects that too, just in a different way.

So useDefineForClassFields: false lets you do this:

// TS visible code
@property() name = 'No one';

However, this must not be initialised as a class field - that's the bit that breaks prototype setter decorators like @property, so the actual JS output (with useDefineForClassFields: false set) is something like:

// JS output
constructor() { 
  this.name = 'No one'; 
}
...
__decorate([property()], ExampleElement.prototype, 'name');

This makes the constructor vs prototype initialisation done by TS something you're relying on. It's not broken (so not bad practice as such) and definitely the easiest course updating existing Lit projects to TS 4.2, but it's also likely to change again in future and potentially have issues with TS's field outputs (they have good reasons for their switch to class fields by default).

Meanwhile declare syntax stops you from initialising fields, but then also doesn't have side effects that are in the JS output but not your TS visible code.

Or to put it another way - if you want to initialise a decorated property with declare you need to set it in the constructor, but this is what TS outputs anyway.

So you can do this explicitly with declare:

// TS visible code
@property() declare name: string;

constructor() { 
  this.name = 'No one'; // You can now set order or break on this with source maps
}

I think it might be better practice to use declare rather than useDefineForClassFields: false because you get fewer potential issues with JS output, but honestly I'm not certain.

Either way declare should be in the documentation, with the explicit caveat that inline initialisation won't work.

Meanwhile useDefineForClassFields: false should have the caveat that inline initialisation will work but won't actually be inline anymore in the output JS.

I've put that in a new PR #583

[aomarks]

// TS visible code
@property() declare name: string;

I would be worried about what this syntax will mean when decorators are finally standardized. This seems like it should be a TypeScript error, because the declare syntax means there would be no class field information in JS for the @property decorator to receive. Thoughts @justinfagnani ?

[justinfagnani]

Wow, declare does seem to partially work: https://www.typescriptlang.org/play?#code/MYewdgzgLgBAZjAvDAFMANhAXDEAjAKwFNgoAaGMAQwFsicAFAJxAAcimoBPAaSK4CUSAHwwA3gF8AUFIxUIEGAEFxUmDAACCACYl0VJkRgAPHGACuNPBwDcUiUA

Declared fields can't have initializers though, but other than that I think all we need from the declaration is the name, which we do get. It might be worth thinking about.

Syntax is definitely doing to change for standard decorators. Authors will have to use the new accessor keyword in the same position as declare actually.