Fix external helper crash after module-status updates

[Copilot]

checkExternalEmitHelpers could panic after an incremental program update when a file changed from script to module. The reused program state could lack the synthetic tslib import specifier needed for helper resolution.

• Program reuse

  • Treat changes to ExternalModuleIndicator or CommonJSModuleIndicator as non-reusable.
  • Forces rebuilt module-resolution state when helper imports may need to be synthesized.

• Regression coverage

  • Added a checker test for updating a file to an exported decorated class under importHelpers.
  • Verifies the updated program rebuilds synthetic tslib state and completes semantic diagnostics without errors.

declare function dec(value: Function): void;

@dec
export class C {}

[DanielRosenwasser]

Perfect - can you write this as a fourslash test instead? Make sure the fourslash test actually fails without the fix.

[DanielRosenwasser]

@copilot

[Copilot]

Perfect - can you write this as a fourslash test instead? Make sure the fourslash test actually fails without the fix.

Addressed in f732c93. I replaced the checker-level regression with a fourslash test and verified it fails without the production fix with the textDocument/diagnostic nil-pointer panic.

[Copilot]

Pull request overview

Note

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

Fixes an incremental-program reuse edge case that could crash external helper resolution when a file changes from script to module (e.g., via adding an export) under importHelpers.

Changes:

• Mark program state as non-reusable when a file’s module-status indicators (ExternalModuleIndicator / CommonJSModuleIndicator) change.
• Add a fourslash regression test that edits a script into an exported decorated module under importHelpers and verifies diagnostics remain error-free.

Show a summary per file

File	Description
internal/fourslash/tests/importHelpersAfterScriptBecomesDecoratedModule_test.go	Adds regression test covering script→decorated module transition with importHelpers.
internal/compiler/program.go	Prevents incremental reuse when module-status indicators change, forcing rebuild of module-resolution/helper state.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0

[jakebailey]

Don't we need to check that they're equal in some way?

[DanielRosenwasser]

I don't think so - my understanding is that these are used to determine if the program is sufficiently structurally the same to avoid rebuilding a full program - not that the files themselves are "equal" before/after. Below, we check that the rest of the structure is equal (e.g. same imports, global modifications, etc.)

From program.go's perspective, the external module indicator probably should not be used to determine anything about the file other than "is this a module, or is this a global?"

[jakebailey]

Actually, I'm not sure I understand why this fix works at all. These are actually a pure function of the file contents + external module indicator options (which is just the file path+reads), which are part of the cache key for files, so when would these change and us not invalidate, or, why would reusing them ever be wrong

[DanielRosenwasser]

I'm not sure which portion you're thinking of, but basically here's what's happening.

First off, every Program includes processedFiles which contains a map of filePath -> synthesizedStringLiteralsToBeUsedAsImportSpecifiers. This seems to me to be calculated at parse-time, but this state is located on the program, not on the file. So as far as I understand, the parse cache wouldn't really come into play here.

Now when a file change comes in, the idea is we try to reuse a lot of the program state for the new Program. UpdateProgram does this via canReplaceFileInProgram. The checks here ensure that we cannot reuse program state related to locating tslib for a script -> module transition.

[Copilot]

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1