Decorators on exported classes

[bradzacher]

@decorator
export class Foo {}

export @decorator class Foo {}

These are both valid syntax in the spec https://github.com/tc39/proposal-decorators?tab=readme-ov-file#syntax

Class decorators may exclusively come before, or after, export/export default.

The difficult thing is handling the locations. The decorator is owned by the class declaration, not the export declaration.

Accordingly the current estree spec https://github.com/estree/estree/blob/master/stage3/decorators.md places the decorator on the class and never the export. Which makes complete sense.

However when one is calculating the "range" / "location" for the class - things get tricky. The class includes the decorator but the decorator lies before the class's parent node. You can't really expand the class's range to include the decorator because that would include the export node as well. But if you don't do that then instead the decorator becomes an orphan of sorts.

I know that estree doesn't technically spec out the location side of things - but given this is something all tools need to implement it bears a discussion at least.

How should this case be handled?
Currently both babel and typescript-eslint go the orphan approach.

[JLHwung]

As for @dec export class C {}, Babel's current approach is to extend the start of the class declaration to the position of @, while its parent node ExportNamedDeclaration still starts with export.

I believe the export @dec class C {} case is not very controversial. So these two cases share the same AST structures. From Babel AST the only way to tell their differences is to check whether the start of the class declaration is smaller than the start of export declaration.

[nzakas]

I wonder if we could simply this a bit by using the orphan approach for both? I'm not a fan of having export inside the bounds of the class declaration, personally, as that complicates inspecting tokens if necessary.

Would it make sense to have the start of a class declaration always be at the class keyword, so a decorator's range is always outside the start of class?

[JLHwung]

Would it make sense to have the start of a class declaration always be at the class keyword, so a decorator's range is always outside the start of class?

As long as we consistently specify that for the class elements' decorators as well, I am personally fine with it, though an element's decorators share the same evaluation scope with its key.

As for now Babel will stay the current approach where a class / class element may start with @, so the general assumption that a node's children will not expand beyond its parent will only be broken in the @dec export class {} case.

[bradzacher]

Yeah this is why I raised it - given this spec is an agreement between tools - I'm happy with whatever solution as long as we agree to implement it that way!

The orphan approach is fine for me because that's how we already implement it! If we want the non-export case to be orphaned too then I'm happy to make that change.

Its probably easier that way as its always consistent then

[nzakas]

The orphan approach is fine for me because that's how we already implement it! If we want the non-export case to be orphaned too then I'm happy to make that change.

Yes, I think this makes sense. The consistency seems important here (at least, to me).

@JLHwung do you think it's possible for Babel to re-evaluate the approach taken before decorators reach stage 4?

[JLHwung]

@nzakas That ship has long sailed. It has been implemented in Babel for a long time anyway, and it supports Babel's transform well. We would like to avoid orphan nodes that start before their parent node. Currently, the exception case in Babel is @dec export class {}, where the ClassDeclaration starts before ExportDeclaration, but the decorators always fit within their parent node.

I also checked the typescript AST: Both decorated class and class element start at @, though they went further to make export a keyword attached to the class declaration, so the typescript AST eliminates the orphan nodes in this case.

[nzakas]

Okay, then I guess that's the way it has to be.

In the future, I recommend we include discussion of source ranges in proposed ESTree additions so we can surface those issues early.

[bradzacher]

Yeah issue is that both babel and TS have had decorators since long before the thought of standardising an experimental AST in here was a twinkle in our eye!

The AST we use now is the same as before, given that the switch from the old, experimental decorators to the new almost standard ones didn't include any real syntax changes.

So we're stuck with the decisions of the past here. It should be possible for typescript-eslint to migrate to babel's approach - it should be a small change to make but a breaking change for a major release.

[nzakas]

Yeah, the history of ESTree is the history of one implementation getting ahead of others and then whoever is most flexible adapts to that. A lot of the weirdness in ESTree is a direct result of this.

Before closing this out, it seems like we should add guidance about the expected syntax ranges to the decorators section?