lib Types and Documentations Fix

[graphemecluster]

Fix #49773
Follow-up fix #29721
Fix #24509 as a side-effect

Besides the fixes from the above issues, this PR also fixes a lot of unclear, missing, or broken documentations (I found that originally some of them are just copied from the spec, without any elaboration). I believe this PR will bring additional enhancements to TypeScript and the community.

For details and the track list about the changes, please see #49773 mentioned above.

---

Fix #17002
Actually fix #32497
Fix #33700
Fix #41808
Actually fix #43247

[ghost]

All CLA requirements met.

[graphemecluster]

Although the number of changed files appears to be large, most of them are just tests with only lib line number or definition changes.
I have carefully checked all of these files. I also created 6 more tests in addition, and 2 were amended with supplements related to the changes.

This PR is now ready for a review.

[P-Daddy]

There are a whole lot of unrelated changes in here. Looking at a smattering of them, it seems that some of them fix minor bugs, some of them are minor usability improvements, and some of them reflect personal preference. I came here from #17002, and I'm having trouble finding the relevant fix for that particular issue among the many other things addressed here.

I've got no stake in the game here, other than as a TypeScript user, but based on my experience, I think the only way you'd get anyone to even consider accepting a change of this magnitude would be if you had a very high reputation with the project stakeholders, and even then, on a large public project like this, with many, many production consumers, it would require careful, timely, and costly review and testing, which would likely be prohibitive given the relatively minor benefits the changes bring.

Please don't think I'm crapping all over your work. Most of what I've looked at so far appear to be good changes, and I appreciate the hard work you've put into them. I'd like to see some of these changes make into a future release. Also, I can relate, because I've certainly been known to push though similar sweeping cleanup efforts in my own projects or those in which I have a high degree of ownership. But in those cases, the risks and costs of mistakes, as well as the time to fix them, were comparatively low. But as we've seen in the past, minor, seemingly obvious changes can sometimes have relatively large unintended consequences, and considering this is one of the top ten most used programming languages in the world, the cost of such consequences can be very high indeed.

I would advise making several, much smaller PRs, each addressing a highly targeted change, so that the risks, benefits, and justifications can be weighed for each individually. I would expect that at least some of them would be accepted in short time, and others might receive helpful feedback to make them even better.

Just my ₿ 0.000042.

[graphemecluster]

Outdated comment

@P-Daddy Thank you for your faithful advices. This PR is dedicated to address #49773. The change to isArray is committed to this PR recently, just because I am confident to this solution. I understand that I shouldn't have done this, so I will separate it into another PR if the Team asks me to do so.

On the other hand, if I split #49773 into multiple PRs by myself, there will be too many – maybe it'll reach 50. This is not a problem, the problem is that I will need to resolve conflict one by one for each opening PR. Doing some maths, 50×51÷2=1275 will be too many for me. (Since the testcases contain information about line numbers in lib files, almost all PRs will conflict with each other.) Therefore, I will only separate PR as per the reviewers' request.

Moreover, in #49773, @RyanCavanaugh commented (#49773 (comment)): (Emphasis added by me)

Some of these seem OK and some are a little bit dicier. We can take a PR and evaluate case-by-case -- please be sure to include testcases that demonstrate what's being fixed.

By the way, I understand that the Team is busy, but is there any timeline as to when this PR will be reviewed? Thanks in advance.

[graphemecluster]

@P-Daddy
OK, because everyone is telling me to separate PR, I had just been exploring some possible ways to split this.
So here they are: #50449, #50450, #50451, #50452, #50453, #50454.

I am not going to close this for the time being, unless the Team wishes.

[graphemecluster]

@P-Daddy
Yes, it seems that benefits do outweigh scale – even such a small change in #42566 takes a year and a half to review.

[fatcerberus]

OK, because everyone is telling me to separate PR

From what I can see only one person asked you to split up the PR, and they're not even a TS maintainer.

[graphemecluster]

@fatcerberus
Because in my past experience, I was told to split every time I open a PR with a considerable size, and I suspect this one is also no exception.
I'm usually reluctant to do this, but if this can speed things up, it’s worth changing my mind to do so.

[fatcerberus]

In that case, for what it’s worth I don’t think the way you’ve split it (part 1, 2, 3, etc.) is useful - the purpose of splitting it should have been so the unrelated changes could be isolated and described individually. Splitting it into “chapters” doesn’t feel like it will make maintainers’ jobs any easier.

Also I noticed in the separated PRs you strongly recommend reviewing this one instead - which entirely defeats the purpose of the split (i.e. allowing the separate smaller parts to be reviewed in isolation).

Just my $0.02 as well - I’m not a TS maintainer either.

[graphemecluster]

Well, the 6 parts can be more or less described individually (I know I should have given them better titles) – there will be line number conflicts after each merge though – and that's the best I could do: to let the Team choose the way they preferred. The recommendation of reviewing this PR is just my personal preference, honestly.
In either way, I am still hoping that I could bring benefits, but not troubles, to the community. My apologies if I did do so.

[graphemecluster]

Great, TypeScript bug discovered! (Temporarily fixed with a non-null assertion)