Skip to content

Fix private properties not being allowed in optional chains#60263

Closed
Psychpsyo wants to merge 3 commits intomicrosoft:mainfrom
Psychpsyo:allow-private-property-in-optional-chain
Closed

Fix private properties not being allowed in optional chains#60263
Psychpsyo wants to merge 3 commits intomicrosoft:mainfrom
Psychpsyo:allow-private-property-in-optional-chain

Conversation

@Psychpsyo
Copy link
Copy Markdown
Contributor

@Psychpsyo Psychpsyo commented Oct 17, 2024

Fixes #42734

This makes it so that expressions of the form foo?.#bar or foo?.bar.#baz are no longer seen as invalid, since the relevant JavaScript standards allow these by now and TypeScript claims to be a superset of JavaScript.
Specifically, private properties are now allowed inside an optional chain.
This syntax is allowed since tc39/proposal-class-fields#301

The added this.a = this; in privateIdentifierChain.1.ts is there so that line 14, this?.a.#b;, does not error because a might be undefined, as that's not what this file is testing for.

I'm sorry for fixing an issue that is not in the backlog, but this is bothering me personally and has existed for years at this point.
Otherwise, I have verified that:

  • Code is up-to-date with the main branch
  • You've successfully run hereby runtests locally
  • There are new or updated unit tests validating the change

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Oct 17, 2024
@Psychpsyo
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@Psychpsyo Psychpsyo changed the title Fix private properties not ebing allowed in optional chains Fix private properties not being allowed in optional chains Oct 17, 2024
@jakebailey jakebailey requested a review from rbuckton January 21, 2025 23:13
Copy link
Copy Markdown
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

Coming back to this, this PR is missing a few things that would make this work:

  • A test that the downlevelling works, so set the test's target to @target: esnext,es2015.
  • A new test that verifies some sort of cross-class error happens, e.g. copy and paste the class in the test to B, and there should be type errors since A is the wrong class.

@github-project-automation github-project-automation bot moved this from Not started to Waiting on author in PR Backlog Jun 30, 2025
@Psychpsyo
Copy link
Copy Markdown
Contributor Author

Coming back to this, this PR is missing a few things that would make this work:

* A test that the downlevelling works, so set the test's target to `@target: esnext,es2015`.

* A new test that verifies some sort of cross-class error happens, e.g. copy and paste the class in the test to `B`, and there should be type errors since `A` is the wrong class.

Done that now.

Comment on lines -6408 to -6410
if (isOptionalChain && isPrivateIdentifier(propertyAccess.name)) {
parseErrorAtRange(propertyAccess.name, Diagnostics.An_optional_chain_cannot_contain_private_identifiers);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suspect we need sort of version check to verify that the syntax is available to the syntax we're emitting, like a new grammar check in the checker to verify this.

Unless you're saying that this syntax has always been allowed since feature was standardized?

@typescript-bot
Copy link
Copy Markdown
Collaborator

With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies).

Next steps for PRs:

  • For crash bugfixes or language service improvements, PRs are currently accepted at the typescript-go repo
  • Changes to type system behavior should wait until after 7.0, at which point mainline TypeScript development will resume in this repository with the Go codebase
  • Library file updates (lib.d.ts etc) continue to live in this repo or the DOM Generator repo as appropriate

@github-project-automation github-project-automation bot moved this from Waiting on author to Done in PR Backlog Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

TS18030: An optional chain cannot contain private identifiers

5 participants