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

add grouped-and-auto-accessors #255

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Sep 9, 2021

View Rendered Text

This PR adds support to the Grouped Accessors and Auto-Accessors proposal.

```js
interface GetAccessorMethod <: Node {
type: "GetAccessorMethod";
params: [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The params must be an empty array. It is to provide compatibility for general function AST visitors: A function always has params property.

}
```

### GetAccessorStub
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use “stub” instead of using the proposal language, such as GetAutoAccessor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "stub" comes from the proposed syntax section.

According to the spec, an auto-accessor is a simplified version of grouped accessor, e.g. accessor x = { get; set; }, where the accessor stub refers to the set and get shorthand.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok.

experimental/grouped-and-auto-accessors.md Outdated Show resolved Hide resolved
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
interface ClassAccessor <: PropertyDefinition {
type: "ClassAccessor";
get: GetAccessorStub | GetAccessorMethod | null;
set: SetAccessorStub | SetAccessorMethod | null;
Copy link

Choose a reason for hiding this comment

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

ClassAccessor should also have a value field which is similar to a class field's value/initializer:

accessor foo { get; set; } = 123;

Copy link

Choose a reason for hiding this comment

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

Ah, I see, it inherits these properties. I think this makes sense.

computed: boolean;
get: GetAccessorStub | GetAccessorMethod | null;
set: SetAccessorStub | SetAccessorMethod | null;
}
Copy link
Contributor Author

@JLHwung JLHwung Sep 19, 2021

Choose a reason for hiding this comment

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

In the latest commit I removed the inheritance from Property, because it will inherit value: Expression and kind: "init" | "set" | "get". These properties are not applicable to the grouped accessor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

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