Fix false-positive TS2354 for native private class field access with importHelpers at dated targets

[Andrew Stegmaier (astegmaier)]

Related to microsoft/TypeScript#63728, and a Go port of the companion fix microsoft/TypeScript#63729.

Disclosure (per CONTRIBUTING.md): This PR was authored with AI assistance (GitHub Copilot CLI). It was directed by me (a specific human operator) investigating and fixing this one specific, previously-filed issue — it is not part of a bulk or queue-driven workflow across issues, and I will personally shepherd it through review and respond to feedback myself.

The bug

With importHelpers: true and a dated target (e.g. ES2022–ES2025), tsgo incorrectly reports TS2354: This syntax requires an imported helper but module 'tslib' cannot be found for plain, fully-native private field/method/accessor access (this.#x, #x in obj, static private fields/methods, private accessors, static blocks, private fields in generics/closures/class expressions/inheritance) — even though the emitted JS for all of these is 100% native ES2022+ syntax that never references tslib.

Root cause

setNodeLinksForPrivateIdentifierScope, checkPropertyAccessExpressionOrQualifiedName, and checkInExpression in internal/checker/checker.go gate the private-identifier helper-requirement check on:

c.languageVersion < LanguageFeatureMinimumTarget.PrivateNamesAndClassStaticBlocks ||
    c.languageVersion < LanguageFeatureMinimumTarget.ClassAndClassElementDecorators ||
    !c.compilerOptions.GetUseDefineForClassFields()

• ClassAndClassElementDecorators is pinned to the ESNext sentinel because TC39 decorators have never been assigned to a dated ECMAScript edition. That makes languageVersion < ClassAndClassElementDecorators true for every dated target, forever — regardless of whether the file uses decorators at all. The actual emitter (estransforms/classfields.go's shouldTransformPrivateElementsOrClassStaticBlocks) only checks languageVersion < ES2022, with no decorator-related gating, so this term in the checker never corresponded to real emit behavior.
• !c.compilerOptions.GetUseDefineForClassFields() has the same problem: private fields are always emitted using "define" semantics regardless of useDefineForClassFields (that option only affects public fields), so it also doesn't correspond to any real difference in whether tslib is needed for private-field access. Verified by emitting with --useDefineForClassFields false --target es2022: output is fully native with no tslib references, yet the checker still errored before this fix.

The fix

Removes both spurious clauses from the 3 call sites above, leaving just c.languageVersion < LanguageFeatureMinimumTarget.PrivateNamesAndClassStaticBlocks.

A 4th call site with the same ClassAndClassElementDecorators check, inside getFirstTransformableStaticClassElement, is intentionally left unchanged. That one only matters when a class is decorated, to decide if the decorator transform's hoisting of static private/static-block elements into an IIFE needs the __setFunctionName helper. I verified (by emitting @dec class C { static #foo() {} } at ES2022) that this really is required in the actual output, so this clause is a genuine, necessary coupling and was left in place.

Testing

• Added testdata/tests/cases/compiler/importHelpersNoHelpersForPrivateFieldsAtES2022.ts — target ES2022, importHelpers: true, no tslib present, covering instance/static private fields, private methods, static private methods, private accessors, private auto-accessors (instance + static), static blocks, and #x in obj. Asserts zero errors; baseline .js confirms the emit never references tslib.
• Added testdata/tests/cases/compiler/importHelpersNoHelpersForPrivateFieldsAtES2022UseDefineForClassFieldsFalse.ts — same coverage with useDefineForClassFields: false, confirming that option no longer triggers the false positive either.
• Ran the full Go test suite (go test ./...); all pass except one unrelated, pre-existing flaky test in internal/fswatch (macOS fsevents timing test, confirmed to fail/pass independently of this change by running it in isolation both with and without the fix).
• Ran the submodule-based conformance suite (TestSubmodule) filtered to privateName|esDecorators|importHelpers|classField|classStaticBlock — all pass, including the decorator+static-private-method case that verifies __setFunctionName is still correctly required.
• hereby lint and hereby format both clean.
• Manually confirmed via the built tsgo binary that decorators and using declarations still correctly require tslib at dated targets (these are real transforms, not part of this bug), and that all "confirmed bug" scenarios from the original repro (https://github.com/astegmaier/typescript-private-field-tslib-repro) now compile cleanly with zero references to tslib in the emitted output.

Update: follow-up fix for a regression found by automated review

The automated Copilot PR reviewer correctly caught a real gap in the fix above: removing the ClassAndClassElementDecorators clause from checkPropertyAccessExpressionOrQualifiedName and checkInExpression went slightly too far. When a class has a native (non-legacy) class decorator and at least one static private/auto-accessor element, decorator lowering (estransforms/esdecorator.go's hasStaticPrivateClassElements/shouldTransformPrivateStaticElementsInClass) hoists all of that class's static private/auto-accessor elements out of the class body into a wrapping closure — which forces accesses to them to use the __classPrivateFieldGet/Set/In helpers from tslib, even at ES2022+. The simplified checks stopped validating that tslib actually exports those helpers in this narrow scenario, which could let an incomplete tslib silently pass type-checking and then fail at runtime. Instance private fields in the same decorated class are unaffected and stay fully native (confirmed by direct JS emission).

I verified this regression directly: reverted to the pre-fix checker, confirmed it correctly errored (TS2343: ... '__classPrivateFieldGet' ... does not exist in 'tslib') for this scenario, then confirmed the (until-now) fixed checker silently accepted it — a real regression, not a false alarm.

The follow-up fix adds isStaticPrivateElementOfDecoratedClass, which mirrors newESDecoratorTransformer's exact skip condition (!legacyDecorators && (target < ESNext || !useDefineForClassFields)), and ORs it back into the two checks (checkPropertyAccessExpressionOrQualifiedName, checkInExpression) that validate the classPrivateField* helpers specifically. setNodeLinksForPrivateIdentifierScope is intentionally left alone — its only consumer (checkWeakMapSetCollision) independently gates on languageVersion <= ES2021, making any change there dead code for this scenario.

Additional verification for the follow-up

• Confirmed via actual JS emission that decorated classes' static private field/method/get-set-accessor/auto-accessor access, and #x in obj checks, really do call tslib_1.__classPrivateFieldGet/Set/In, while instance private field access in the same decorated class stays fully native — in every target/option combination that matters (es2022; esnext with useDefineForClassFields true and false; legacy experimentalDecorators, where this combination is grammatically forbidden anyway).
• Dispatched two independent review passes (one focused on tracing the fix against the emitter source and edge cases; one focused on writing fresh, independent test matrices and directly diffing checker behavior against real JS emission across targets/decorators/element-kinds/access-kinds). Investigated and ruled out two additional discrepancies they surfaced (a missing TS2354 for the decorator's own helpers, and an emit panic for static auto-accessors — both specific to target: esnext with useDefineForClassFields: false): both are confirmed pre-existing in main, unrelated to any change in this branch, and out of scope for this fix.
• Ran the full Go test suite (go test ./...) and the submodule conformance suite filtered to privateName|esDecorators|importHelpers|classField|classStaticBlock — all passing.
• hereby lint and hereby format clean.

New tests

• importHelpersRequiredForDecoratedStaticPrivateElements.ts — a decorated class with an instance field plus static field/method/get-set-accessor/auto-accessor and a static #x in obj check, with a tslib stub that provides the decorator helpers but omits the classPrivateField* ones. Confirms exactly the 3 expected errors (one per helper), anchored at the static accesses, with zero errors for the instance field.
• importHelpersRequiredForDecoratedStaticPrivateElementsUseDefineForClassFieldsFalse.ts — same shape at target: esnext with useDefineForClassFields: false, confirming decorator lowering (and the helper requirement) still applies in that specific combination.
• importHelpersNotRequiredForDecoratedStaticPrivateElementsAtESNext.ts — the same shape at target: esnext with the default useDefineForClassFields: true, confirming zero errors and fully native emission when decorator lowering is skipped entirely.

[Andrew Stegmaier (astegmaier)]

Ryan Cavanaugh (@RyanCavanaugh) - this was the bug fix that we just discussed.

Disclosure: I put a lot of human effort into making the simplified repro for the original bug (https://github.com/astegmaier/typescript-private-field-tslib-repro). But this PR was agent-made -- by pointing it at the reproduction -- but it seems simple and sensible enough (really just three small lines) that it was worth submitting.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/checker/checker.go:11272

• Decorator lowering is another path that emits these helpers even at ES2022+. internal/transformers/estransforms/esdecorator.go:632-635 marks class-decorated classes with static private elements for forced transformation, and classfields.go:1064-1071 / 1495-1518 then rewrites their reads and writes to __classPrivateFieldGet/__classPrivateFieldSet. With this target-only guard, the checker no longer validates those imported helpers, so a partial or incompatible tslib can type-check but fail in emitted code. Please retain helper checking when decorator lowering forces the referenced static private element to transform, and add a decorated-static-private regression case.
  internal/checker/checker.go:13083
• This also drops required helper validation for #x in value when #x is static and its class is class-decorated. The decorator transform sets EFTransformPrivateStaticElements regardless of target (esdecorator.go:632-635), after which classfields.go:1606-1608 emits __classPrivateFieldIn. At ES2022+ this guard now skips checking that tslib exports the emitted helper. Please include the decorator-forced static-private path in this condition as well.

[Andrew Stegmaier (astegmaier)]

In response to copilot feedback (agent generated):

I verified this was a real regression (confirmed via actual JS emission that decorated classes' static private elements do call tslib_1.__classPrivateFieldGet/Set/In, and that the pre-follow-up checker silently stopped validating for it) and pushed a fix that restores the check specifically for that scenario (native class decorator + static private/auto-accessor element), while keeping the original fix's improvement for the common case (plain classes, instance fields). Added 3 new tests covering it. See the updated PR description for details.