Support decorator auto accessors syntax #13919

sosukesuzuki · 2022-11-30T03:41:16Z

Description

Partial of #13516

Note 1:

babel-ts does not support auto accessors with type annotations yet (babel/babel#15209), so tests for it is in misc/typescript-only.

Note 2:

Edit: reported to babel babel/babel#15238

babel-ts parses

class Foo {
  accessor
  foo
}

to the AST like

{
  type: class
  children: [
    property ( name = "accessor" ),
    property ( name = "foo" )
  ]
}

but typescript parses to the AST like

{
  type: class
  children: [
    accessor property ( name = "foo" )
  ]
}

so, we have placed separate tests for it in format/misc/typescript-only and format/misc/typescript-babel-only, respectively.

Checklist

I’ve added tests to confirm my change works.
(If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
(If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
I’ve read the contributing guidelines.

✨Try the playground for this PR✨

changelog_unreleased/typescript/13919.md

fisker · 2022-11-30T04:31:23Z

So, should we insert ; if member is accessor?

Prettier pr-13919
Playground link

--parser typescript
--no-semi

Input:

class Foo {
  accessor;
  foo = 1
}

Output:

class Foo {
  accessor
  foo = 1
}

Second Output:

class Foo {
  accessor foo = 1
}

fisker · 2022-11-30T04:34:03Z

prettier/src/language-js/print/statement.js

Line 180 in 655a161

(name === "static" || name === "get" || name === "set") &&

src/language-js/print/statement.js

Co-authored-by: fisker Cheung <lionkay@gmail.com>

sosukesuzuki · 2022-12-05T07:51:22Z

rebased

fisker · 2022-12-05T08:56:14Z

src/language-js/print/statement.js

+    (name === "static" ||
+      name === "get" ||
+      name === "set" ||
+      name === "accessor") &&


Please add a TODO comment about microsoft/TypeScript#51707

fisker · 2022-12-05T09:05:02Z

src/language-js/print/statement.js

-  type === "ClassAccessorProperty";
+  type === "ClassAccessorProperty" ||
+  type === "AccessorProperty" ||
+  type === "TSAbstractAccessorProperty";


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

My guess is it's missed.

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

What a TSAbstractPropertyDefinition look like

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

?

We can test it.

Yes, missed playground, playground 2

fisker · 2022-12-05T09:07:38Z

src/language-js/print/statement.js

-  type === "ClassAccessorProperty";
+  type === "ClassAccessorProperty" ||
+  type === "AccessorProperty" ||
+  type === "TSAbstractAccessorProperty";
 /**
 * @returns {boolean}
 */
 function shouldPrintSemicolonAfterClassProperty(node, nextNode) {
  const name = node.key && node.key.name;


Since we are here. Let's remove this chaining, I don't think the key can be nullish here.

Suggested change

const name = node.key && node.key.name;

const { name } = node.key;

PraveenNanda124

Good work

fisker · 2022-12-05T10:34:04Z

src/language-js/print/statement.js

  if (
    (name === "static" ||
      name === "get" ||
      name === "set" ||
+      // TODO: Remove this https://github.com/microsoft/TypeScript/issues/51707 is fixed
      name === "accessor") &&
    !node.value &&
    !node.typeAnnotation


printClassProperty also prints ? or ! here

prettier/src/language-js/print/class.js

Line 229 in aa34209

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.

We are not checking computed either...

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [prettier](https://prettier.io) ([source](https://github.com/prettier/prettier)) | devDependencies | patch | [`2.8.0` -> `2.8.1`](https://renovatebot.com/diffs/npm/prettier/2.8.0/2.8.1) | --- ### Release Notes <details> <summary>prettier/prettier</summary> ### [`v2.8.1`](https://github.com/prettier/prettier/blob/HEAD/CHANGELOG.md#281) [Compare Source](prettier/prettier@2.8.0...2.8.1) [diff](prettier/prettier@2.8.0...2.8.1) ##### Fix SCSS map in arguments ([#9184](prettier/prettier#9184) by [@agamkrbit](https://github.com/agamkrbit))  ```scss // Input $display-breakpoints: map-deep-merge( ( "print-only": "only print", "screen-only": "only screen", "xs-only": "only screen and (max-width: #{map-get($grid-breakpoints, "sm")-1})", ), $display-breakpoints ); // Prettier 2.8.0 $display-breakpoints: map-deep-merge( ( "print-only": "only print", "screen-only": "only screen", "xs-only": "only screen and (max-width: #{map-get($grid-breakpoints, " sm ")-1})", ), $display-breakpoints ); // Prettier 2.8.1 $display-breakpoints: map-deep-merge( ( "print-only": "only print", "screen-only": "only screen", "xs-only": "only screen and (max-width: #{map-get($grid-breakpoints, "sm")-1})", ), $display-breakpoints ); ``` ##### Support auto accessors syntax ([#13919](prettier/prettier#13919) by [@sosukesuzuki](https://github.com/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)  ```tsx class Foo { accessor foo: number = 3; } ``` </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1671 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

* Install new ts-estree * Print auto accessor properties * Support edge cases * Misc tests * Add tests for auto accessor with type annotations * Add changelog * Update changelog_unreleased/typescript/13919.md Co-authored-by: fisker Cheung <lionkay@gmail.com> * Update changelog_unreleased/typescript/13919.md Co-authored-by: fisker Cheung <lionkay@gmail.com> * Print semicolon for accessor and add tests * Add TODO comment * Add to properties list * Address review Co-authored-by: fisker Cheung <lionkay@gmail.com>

fisker reviewed Nov 30, 2022

View reviewed changes

changelog_unreleased/typescript/13919.md Outdated Show resolved Hide resolved

fisker reviewed Nov 30, 2022

View reviewed changes

changelog_unreleased/typescript/13919.md Outdated Show resolved Hide resolved

fisker reviewed Nov 30, 2022

View reviewed changes

src/language-js/print/statement.js Show resolved Hide resolved

sosukesuzuki mentioned this pull request Nov 30, 2022

[Bug]: Inconsistently parsing between TypeScript and Babel for decorator auto accessors babel/babel#15238

Closed

1 task

This was referenced Dec 1, 2022

Add tests for keyword class property #13927

Merged

Update typescript-eslint to v5.45.0 #13947

Closed

sosukesuzuki and others added 9 commits December 5, 2022 16:50

Install new ts-estree

4555d4b

Print auto accessor properties

7aa403f

Support edge cases

ba35f42

Misc tests

c6723fc

Add tests for auto accessor with type annotations

84fde47

Add changelog

a48cf7f

Update changelog_unreleased/typescript/13919.md

34770f2

Co-authored-by: fisker Cheung <lionkay@gmail.com>

Update changelog_unreleased/typescript/13919.md

929c2dd

Co-authored-by: fisker Cheung <lionkay@gmail.com>

Print semicolon for accessor and add tests

c641b96

sosukesuzuki force-pushed the support-auto-accessor branch from e094ef3 to c641b96 Compare December 5, 2022 07:50

fisker reviewed Dec 5, 2022

View reviewed changes

PraveenNanda124 reviewed Dec 5, 2022

View reviewed changes

sosukesuzuki added 3 commits December 5, 2022 18:26

Add TODO comment

604fc53

Add to properties list

9055783

Address review

45247b5

fisker reviewed Dec 5, 2022

View reviewed changes

fisker approved these changes Dec 5, 2022

View reviewed changes

sosukesuzuki merged commit 1cf760a into prettier:main Dec 5, 2022

sosukesuzuki deleted the support-auto-accessor branch December 5, 2022 11:50

fisker mentioned this pull request Dec 15, 2022

Improve keyword class property print #13997

Merged

4 tasks

github-actions bot mentioned this pull request Jul 9, 2023

Upgrade renovate to get pnpm dependencies reported correctly AurorNZ/report-updated-dependencies#661

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support decorator auto accessors syntax #13919

Support decorator auto accessors syntax #13919

sosukesuzuki commented Nov 30, 2022 •

edited

fisker commented Nov 30, 2022 •

edited

fisker commented Nov 30, 2022

sosukesuzuki commented Dec 5, 2022

fisker Dec 5, 2022

fisker Dec 5, 2022 •

edited

fisker Dec 5, 2022 •

edited

sosukesuzuki Dec 5, 2022

fisker Dec 5, 2022

fisker Dec 5, 2022 •

edited

fisker Dec 5, 2022 •

edited

PraveenNanda124 left a comment

fisker Dec 5, 2022

fisker Dec 5, 2022

	const name = node.key && node.key.name;
	const { name } = node.key;

Navigation Menu

Support decorator auto accessors syntax #13919

Support decorator auto accessors syntax #13919

Conversation

sosukesuzuki commented Nov 30, 2022 • edited

Description

Checklist

fisker commented Nov 30, 2022 • edited

fisker commented Nov 30, 2022

sosukesuzuki commented Dec 5, 2022

fisker Dec 5, 2022

Choose a reason for hiding this comment

fisker Dec 5, 2022 • edited

Choose a reason for hiding this comment

fisker Dec 5, 2022 • edited

Choose a reason for hiding this comment

sosukesuzuki Dec 5, 2022

Choose a reason for hiding this comment

fisker Dec 5, 2022

Choose a reason for hiding this comment

fisker Dec 5, 2022 • edited

Choose a reason for hiding this comment

fisker Dec 5, 2022 • edited

Choose a reason for hiding this comment

PraveenNanda124 left a comment

Choose a reason for hiding this comment

fisker Dec 5, 2022

Choose a reason for hiding this comment

fisker Dec 5, 2022

Choose a reason for hiding this comment

sosukesuzuki commented Nov 30, 2022 •

edited

fisker commented Nov 30, 2022 •

edited

fisker Dec 5, 2022 •

edited

fisker Dec 5, 2022 •

edited

fisker Dec 5, 2022 •

edited

fisker Dec 5, 2022 •

edited