Skip to content

fix: Inconsistencies in ESM-style imports of accessibility-modified proper...#63518

Closed
arnavnagzirkar wants to merge 2 commits into
microsoft:mainfrom
arnavnagzirkar:fix-62519
Closed

fix: Inconsistencies in ESM-style imports of accessibility-modified proper...#63518
arnavnagzirkar wants to merge 2 commits into
microsoft:mainfrom
arnavnagzirkar:fix-62519

Conversation

@arnavnagzirkar
Copy link
Copy Markdown

Summary

Fix for issue #62519.

Issue

Fixes #62519

Issue URL: #62519

Changes

AGENTS.md                                          | 59 +++++++++++-----------
 src/compiler/checker.ts                            |  7 ++-
 .../namedImportFromCjsClassExportAccessibility.ts  | 15 ++++++
 3 files changed, 50 insertions(+), 31 deletions(-)

Testing

  • Agent ran relevant tests during development

  • Linting checks passed

  • Changes are minimal and focused on the issue

Copilot AI review requested due to automatic review settings June 1, 2026 00:45
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Jun 1, 2026
@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jun 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an accessibility check for named imports from CommonJS export = modules when the export is a class and the imported names map to static class members.

Changes:

  • Introduces a new compiler test covering named imports from export = class modules with differing member visibility.
  • Updates the checker to validate accessibility for import { … } from "./cjsExportEqualsClass" when those names resolve to class members.

Reviewed changes

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

File Description
tests/cases/compiler/namedImportFromCjsClassExportAccessibility.ts Adds a regression test for importing static class members (public/protected/private) from a CJS export = class.
src/compiler/checker.ts Adds an accessibility check when resolving named imports against export = module exports that are class members.
Comments suppressed due to low confidence (3)

src/compiler/checker.ts:1

  • checkPropertyAccessibilityAtLocation reports diagnostics as a side effect. Calling it from this symbol-resolution path can produce duplicate diagnostics or diagnostics in contexts that only intended to query symbols (e.g., language service queries that reuse checker helpers). Prefer moving the accessibility diagnostic emission into the dedicated import-checking path (where other import diagnostics are produced) and keep this helper focused on returning/resolving symbols.
    src/compiler/checker.ts:1
  • The diagnostic location is currently the whole ImportSpecifier node (specifier). To make the error squiggle more precise (and consistent with other property-accessibility errors), pass the imported name node (e.g., name / specifier.name as appropriate) as the location argument so the diagnostic points at the specific inaccessible import (b / c) rather than the entire specifier.
    src/compiler/checker.ts:1
  • getTypeOfSymbol(targetSymbol) is called multiple times in this block (also at line 4083). Consider storing it in a local (e.g., const targetType = getTypeOfSymbol(targetSymbol);) and reusing it for both getPropertyOfType(...) and checkPropertyAccessibilityAtLocation(...) to avoid repeated work in hot paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@RyanCavanaugh
Copy link
Copy Markdown
Member

#63521 (comment)

@github-project-automation github-project-automation Bot moved this from Not started to Done in PR Backlog Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Backlog Bug PRs that fix a backlog bug

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Inconsistencies in ESM-style imports of accessibility-modified properties from CJS-exported classes

4 participants