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

Support decorator auto accessors syntax #13919

Merged
merged 12 commits into from Dec 5, 2022
12 changes: 12 additions & 0 deletions changelog_unreleased/typescript/13919.md
@@ -0,0 +1,12 @@
#### Support auto accessors syntax (#13919 by @sosukesuzuki)

Support for [Auto Accessors Syntax](https://devblogs.microsoft.com/typescript/announcing-typescript-4-9/#auto-accessors-in-classes) landed in TypeScript 4.9.

(Doesn't work well with `babel-ts` parser)

<!-- prettier-ignore -->
```tsx
class Foo {
accessor foo: number = 3;
}
```
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -27,7 +27,7 @@
"@babel/parser": "7.20.1",
"@glimmer/syntax": "0.84.2",
"@iarna/toml": "2.2.5",
"@typescript-eslint/typescript-estree": "5.44.0",
"@typescript-eslint/typescript-estree": "5.45.0",
"acorn": "8.8.0",
"acorn-jsx": "5.3.2",
"angular-estree-parser": "2.5.1",
Expand Down
12 changes: 10 additions & 2 deletions src/language-js/print/class.js
Expand Up @@ -209,7 +209,11 @@ function printClassProperty(path, options, print) {
if (node.static) {
parts.push("static ");
}
if (node.type === "TSAbstractPropertyDefinition" || node.abstract) {
if (
node.type === "TSAbstractPropertyDefinition" ||
node.type === "TSAbstractAccessorProperty" ||
node.abstract
) {
parts.push("abstract ");
}
if (node.override) {
Expand All @@ -221,7 +225,11 @@ function printClassProperty(path, options, print) {
if (node.variance) {
parts.push(print("variance"));
}
if (node.type === "ClassAccessorProperty") {
if (
node.type === "ClassAccessorProperty" ||
node.type === "AccessorProperty" ||
node.type === "TSAbstractAccessorProperty"
) {
parts.push("accessor ");
}
parts.push(
Expand Down
13 changes: 10 additions & 3 deletions src/language-js/print/statement.js
Expand Up @@ -168,14 +168,21 @@ const isClassProperty = ({ type }) =>
type === "ClassProperty" ||
type === "PropertyDefinition" ||
type === "ClassPrivateProperty" ||
type === "ClassAccessorProperty";
type === "ClassAccessorProperty" ||
type === "AccessorProperty" ||
type === "TSAbstractPropertyDefinition" ||
type === "TSAbstractAccessorProperty";
Copy link
Sponsor Member

@fisker fisker Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you investigate why TSAbstractPropertyDefinition not listed here, or why TSAbstractAccessorProperty should be added here?

Copy link
Sponsor Member

@fisker fisker Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is it's missed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I add "TSAbstractPropertyDefinition" to this list, the test does not fail. So it is maybe missed.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a TSAbstractPropertyDefinition look like

class A {
	abstract get;
	foo() {}
}

?

We can test it.

Copy link
Sponsor Member

@fisker fisker Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, missed playground, playground 2

/**
* @returns {boolean}
*/
function shouldPrintSemicolonAfterClassProperty(node, nextNode) {
const name = node.key && node.key.name;
const { name } = node.key;
if (
(name === "static" || name === "get" || name === "set") &&
(name === "static" ||
fisker marked this conversation as resolved.
Show resolved Hide resolved
name === "get" ||
name === "set" ||
// TODO: Remove this https://github.com/microsoft/TypeScript/issues/51707 is fixed
name === "accessor") &&
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a TODO comment about microsoft/TypeScript#51707

!node.value &&
!node.typeAnnotation
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printClassProperty also prints ? or ! here

printOptionalToken(path),
, not sure if we can omit semi here if those tokens exists, but we can investigate in feature.

We should also test private fields, since we use .key.name directly without check type.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not checking computed either...

) {
Expand Down
1 change: 1 addition & 0 deletions src/language-js/print/typescript.js
Expand Up @@ -115,6 +115,7 @@ function printTypescript(path, options, print) {
case "TSAbstractMethodDefinition":
case "TSDeclareMethod":
return printClassMethod(path, options, print);
case "TSAbstractAccessorProperty":
case "TSAbstractPropertyDefinition":
return printClassProperty(path, options, print);
case "TSInterfaceHeritage":
Expand Down
3 changes: 3 additions & 0 deletions src/language-js/printer-estree.js
Expand Up @@ -98,6 +98,8 @@ function genericPrint(path, options, print, args) {
type === "ClassPrivateMethod" ||
type === "ClassProperty" ||
type === "ClassAccessorProperty" ||
type === "AccessorProperty" ||
type === "TSAbstractAccessorProperty" ||
type === "PropertyDefinition" ||
type === "TSAbstractPropertyDefinition" ||
type === "ClassPrivateProperty" ||
Expand Down Expand Up @@ -766,6 +768,7 @@ function printPathNoParens(path, options, print, args) {
case "PropertyDefinition":
case "ClassPrivateProperty":
case "ClassAccessorProperty":
case "AccessorProperty":
return printClassProperty(path, options, print);
case "TemplateElement":
return replaceTextEndOfLine(node.value.raw);
Expand Down