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

Remove incorrect statement in Classes.md #2695

Closed
wants to merge 1 commit into from

Conversation

ericmutta
Copy link

Remove the following line:

Getters and setters must have the same Member Visibility

...because it is not true. The following code proves that getters and setters can have different visibility when run in the TypeScript Playground:

class C {
  _length = 0;
  public get length() { // here the getter is public.
    return this._length;
  }
  private set length(value) { // here the setter is private.
    this._length = value;
  }
}

let c: C = new C();
let n = c.length; // this works bcause getter is public.
c.length = 12; // compiler complains here because setter is private.

Remove the following line:

> Getters and setters must have the same [Member Visibility](#member-visibility)

...because it is not true. The following code proves this [when run in the TypeScript Playground](https://www.typescriptlang.org/play?#code/MYGwhgzhAEDC0G8CwAoa0D6ICmA7A5gC4AW0AvNAAwDcq6ADgK4BGIAlsNPtodDgSQAUASkTQA9OOjFsAJ2zQSC7oUJzobGE1YcAdHXTR5hRrNyLim3VjxFitNNAC+B+rLYA3MGugQefWyEvEEZsUQQJKRl5CwU-VXVNaDdPb2x9R3QSKxsBUgpg0Id0FxRS1BxeYAAuOHJoXGwAdzgRB0qG+uBdfjtqSIskpoB7WQBrGGZgMEY-Lh41WQ0tFnZu1G7eknqARgAmfsloYGGAW3o2HCWT8-A2XBhohWZsadm4hcStdy81XSA):

```ts
class C {
  _length = 0;
  public get length() { // here the getter is public.
    return this._length;
  }
  private set length(value) { // here the setter is private.
    this._length = value;
  }
}

let c: C = new C();
let n = c.length; // this works bcause getter is public.
c.length = 12; // compiler complains here because setter is private.
```
@ericmutta
Copy link
Author

ericmutta commented Feb 5, 2023

Hello @orta as someone who contributed to the same file in this PR, would you happen to know who currently monitors contributions to the TypeScript website?

@orta
Copy link
Contributor

orta commented Feb 6, 2023

The team hasn't made quite figured out how to handle them, #2585

That said this PR gets a 👍🏻 from me, that change is relatively recent and it'd be good to remove that line

@ericmutta
Copy link
Author

Many thanks @orta for the link to that issue, it helped clarify the situation 👍

PS: thanks too for all the effort you personally invested in the TypeScript website. It has proven to be a very valuable resource in learning what is arguably one of the most insanely powerful languages in mainstream use today 🔥

@ericmutta
Copy link
Author

Closing since no one seems to follow up on PRs for this repo 👎

@ericmutta ericmutta closed this Apr 8, 2023
@agilgur5
Copy link
Contributor

fwiw, PR reviews are now being resumed as of #2804. it took a year for some of my PRs to get reviewed and merged, but today some of mine got merged within minutes.

I stopped contributing to the website for a good given the lack of follow-up / maintenance, so I understand the frustration too 😕 I'm glad it's being maintained more actively again now though!

@ericmutta
Copy link
Author

Thanks @agilgur5 for the pointer to #2804 it's encouraging to see the team is doing something about it 👍

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.

3 participants