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

Conversation

sosukesuzuki
Copy link
Member

@sosukesuzuki sosukesuzuki commented Nov 30, 2022

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

@fisker
Copy link
Member

fisker commented Nov 30, 2022

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
Copy link
Member

fisker commented Nov 30, 2022

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

@sosukesuzuki
Copy link
Member Author

rebased

(name === "static" ||
name === "get" ||
name === "set" ||
name === "accessor") &&
Copy link
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

type === "ClassAccessorProperty";
type === "ClassAccessorProperty" ||
type === "AccessorProperty" ||
type === "TSAbstractAccessorProperty";
Copy link
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
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
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
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

type === "ClassAccessorProperty";
type === "ClassAccessorProperty" ||
type === "AccessorProperty" ||
type === "TSAbstractAccessorProperty";
/**
* @returns {boolean}
*/
function shouldPrintSemicolonAfterClassProperty(node, nextNode) {
const name = node.key && node.key.name;
Copy link
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.

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;

Copy link

@PraveenNanda124 PraveenNanda124 left a comment

Choose a reason for hiding this comment

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

Good work

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
Copy link
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
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...

@sosukesuzuki sosukesuzuki merged commit 1cf760a into prettier:main Dec 5, 2022
@sosukesuzuki sosukesuzuki deleted the support-auto-accessor branch December 5, 2022 11:50
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Dec 23, 2022
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#&#8203;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 ([#&#8203;9184](prettier/prettier#9184) by [@&#8203;agamkrbit](https://github.com/agamkrbit))

<!-- prettier-ignore -->

```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 ([#&#8203;13919](prettier/prettier#13919) by [@&#8203;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)

<!-- prettier-ignore -->

```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.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC41MS4wIiwidXBkYXRlZEluVmVyIjoiMzQuNzAuNCJ9-->

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>
medikoo pushed a commit to medikoo/prettier-elastic that referenced this pull request Jan 4, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants