fix: remove false tslib requirement for native #private fields at ES2022+ - #63751
Closed
Erfan (ErfanBagheri404) wants to merge 1 commit into
Closed
fix: remove false tslib requirement for native #private fields at ES2022+#63751Erfan (ErfanBagheri404) wants to merge 1 commit into
Erfan (ErfanBagheri404) wants to merge 1 commit into
Conversation
…022+ Fixes microsoft#63728 The checkExternalEmitHelpers conditions for private identifier access included `ClassAndClassElementDecorators` as an OR term, which is ESNext (decorators aren't in any dated ES spec). This made the entire condition always true for dated targets (ES2022-ES2025), incorrectly requiring tslib for native #private field access that needs no helpers. Remove the ClassAndClassElementDecorators and !useDefineForClassFields terms from the three private-identifier helper-gate conditions in checker.ts, leaving only the PrivateNamesAndClassStaticBlocks check (ES2022).
Author
|
@microsoft-github-policy-service agree |
Member
|
The TypeScript repo is closed for development; PR should be in the typescript-go repo. Please see the top section of CONTRIBUTING.md and pinned issue #62963. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an incorrect TS2354 (“requires an imported helper… tslib”) being reported for native #private class member usage when importHelpers: true and target >= ES2022, even though the emitter doesn’t downlevel those constructs or emit tslib helpers at those targets.
Changes:
- Simplifies three private-identifier
checkExternalEmitHelpersgate conditions inchecker.tsto depend only onPrivateNamesAndClassStaticBlocks(ES2022). - Adds new compiler test cases for ES2022 (including static blocks and auto-accessors) and an ES2022 +
useDefineForClassFields: falsevariant. - Adds baselines verifying JS output contains no
tslibimports/helpers for these scenarios.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/compiler/checker.ts | Removes always-true (for dated targets) helper-gating terms so ES2022+ native private members don’t spuriously require tslib. |
| tests/cases/compiler/importHelpersNoHelpersForPrivateFieldsAtES2022.ts | New regression test covering many ES2022-native/private-member forms under importHelpers: true. |
| tests/cases/compiler/importHelpersNoHelpersForPrivateFieldsAtES2022UseDefineForClassFieldsFalse.ts | New regression test ensuring ES2022 behavior remains correct with useDefineForClassFields: false. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022.js | Baseline confirming no tslib import/helper usage for the ES2022 test. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022.types | Type baseline for the ES2022 test. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022.symbols | Symbol baseline for the ES2022 test. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022UseDefineForClassFieldsFalse.js | Baseline confirming no tslib import/helper usage for the ES2022 + useDefineForClassFields: false test. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022UseDefineForClassFieldsFalse.types | Type baseline for the ES2022 + useDefineForClassFields: false test. |
| tests/baselines/reference/importHelpersNoHelpersForPrivateFieldsAtES2022UseDefineForClassFieldsFalse.symbols | Symbol baseline for the ES2022 + useDefineForClassFields: false test. |
Suppressed comments (1)
src/compiler/checker.ts:34899
- The tslib-helper check for private-field writes is still unconditional for destructuring assignments:
checkReferenceAssignmentcallscheckExternalEmitHelpers(..., ClassPrivateFieldSet)whenever the target is a private-identifier property access, without checkinglanguageVersion. This can still produce TS2354 attarget >= ES2022for patterns like({ a: this.#x } = obj)even though the emitter only needs__classPrivateFieldSetwhen downleveling private elements (target < ES2022). Consider gating that call with the samelanguageVersion < LanguageFeatureMinimumTarget.PrivateNamesAndClassStaticBlockscondition, and adding a regression test that assigns intothis.#fieldvia destructuring undertarget: es2022+importHelpers: true.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closed
This PR targets the JavaScript-based TypeScript codebase, which is in maintenance mode. The fix has been ported to
microsoft/typescript-goin PR #4841 by astegmaier.Closing in favor of the Go-based fix.