Add missing TS6504 diagnostic for JavaScript files without allowJs #2104

Copilot · 2025-11-15T19:22:13Z

Fix missing error TS6504 when JS file compiled without allowJs

Understand the issue: tsgo is missing error TS6504 when compiling JS files without allowJs
Locate where root files are filtered in fileloader.go (addRootTask function)
Add diagnostic when JS file is encountered without allowJs being enabled
Move check to common path (parseTask.load) to handle all file types
Test the fix with checkJsFiles6 and other test cases
Run the full test suite to ensure no regressions
Format and lint the code
Verify manual testing with tsgo binary matches tsc behavior for both root files and references
Fix data race in includeProcessor

Summary

This PR fixes the issue where tsgo was not reporting error TS6504 when compiling JavaScript files without the allowJs option enabled.

Changes:

Added diagnostic check in parseTask.load() in filesparser.go to detect JS files without allowJs
Modified addRootTask() to include files with extensions even if unsupported, so they can be checked in load()
Check now applies to all file inclusion paths: root files, triple-slash references, type references, etc.
Fixed data race by adding mutex protection to includeProcessor for concurrent access to diagnostic and file reason maps

Testing:

✅ Root file: tsgo a.js now reports error TS6504
✅ Triple-slash ref: /// <reference path="helper.js" /> now reports error TS6504
✅ All existing tests pass including the previously failing bundlerImportTsExtensions test
✅ Manual testing confirms behavior matches tsc
✅ Race detector passes with no data races

Original prompt

This section details on the original issue you should resolve

<issue_title>Missing error when JS file compiled without allowJs on</issue_title>
<issue_description>## Steps to reproduce


D:\Throwaway\corsarepro>tsc a.js
error TS6504: File 'a.js' is a JavaScript file. Did you mean to enable the 'allowJs' option?
  The file is in the program because:
    Root file specified for compilation


Found 1 error.


D:\Throwaway\corsarepro>tsgo a.js

D:\Throwaway\corsarepro>
```</issue_description>

<agent_instructions>Use print debugging and `go test -v` to trace why the file is not being included or errored on for the test case diff mentioned in the issue thread. Print debugging with the tsgo binary on the example in the issue itself may also work.</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@jakebailey</author><body>
I assume this is https://github.com/microsoft/typescript-go/blob/05f4d69f81d416946ce46a374d91b78c0fbd37c8/testdata/baselines/reference/submodule/compiler/checkJsFiles6.errors.txt.diff#L5</body></comment_new>
</comments>

Fixes Missing error when JS file compiled without allowJs on #2084

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: jakebailey <[email protected]>

jakebailey

I think this seems fine?

Copilot

Pull Request Overview

This PR adds the missing TS6504 diagnostic for JavaScript files compiled without the allowJs option enabled. Previously, tsgo silently ignored these files while tsc reports a helpful error message, making tsgo less user-friendly.

Key Changes

Modified internal/compiler/fileloader.go to detect JavaScript files and report TS6504 diagnostic when allowJs is not enabled
Updated 26 test baseline files to reflect the new behavior, showing convergence with TypeScript's behavior

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/compiler/fileloader.go	Added logic to detect JS files without allowJs and report TS6504 diagnostic with file inclusion context
testdata/baselines/reference/submodule/compiler/checkJsFiles6.errors.txt.diff	Removed diff showing convergence - now properly reports TS6504 for JS files
testdata/baselines/reference/submodule/compiler/checkJsFiles6.errors.txt	Added TS6504 error alongside existing TS5052 error
testdata/baselines/reference/submodule/compiler/jsFileCompilationWithoutJsExtensions.errors.txt.diff	Removed diff showing convergence
testdata/baselines/reference/submodule/compiler/jsFileCompilationWithoutJsExtensions.errors.txt	Added TS6504 error for JS file compilation
testdata/baselines/reference/submodule/conformance/nodeModulesAtTypesPriority.errors.txt.diff	Removed diff showing convergence for node_modules JS files
testdata/baselines/reference/submodule/conformance/nodeModulesAtTypesPriority.errors.txt	Added TS6504 errors for JS files in node_modules
testdata/baselines/reference/submodule/conformance/bundlerNodeModules1(module=preserve).errors.txt.diff	Removed diff for bundler module resolution
testdata/baselines/reference/submodule/conformance/bundlerNodeModules1(module=preserve).errors.txt	Added TS6504 errors for .cjs and .js files
testdata/baselines/reference/submodule/conformance/bundlerNodeModules1(module=esnext).errors.txt.diff	Removed diff for esnext module
testdata/baselines/reference/submodule/conformance/bundlerNodeModules1(module=esnext).errors.txt	Added TS6504 errors for bundler with esnext
testdata/baselines/reference/submodule/conformance/bundlerConditionsExcludesNode(module=preserve).errors.txt.diff	Removed diff for bundler conditions
testdata/baselines/reference/submodule/conformance/bundlerConditionsExcludesNode(module=preserve).errors.txt	Added TS6504 errors for .node.js and .web.js files
testdata/baselines/reference/submodule/conformance/bundlerConditionsExcludesNode(module=esnext).errors.txt.diff	Removed diff for esnext with bundler conditions
testdata/baselines/reference/submodule/conformance/bundlerConditionsExcludesNode(module=esnext).errors.txt	Added TS6504 errors for conditional JS files
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node16).errors.txt.diff	Removed diff for node16 module
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node16).errors.txt	Added TS6504 error for exports with node16
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node18).errors.txt.diff	Removed diff for node18 module
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node18).errors.txt	Added TS6504 error for exports with node18
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node20).errors.txt.diff	Removed diff for node20 module
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=node20).errors.txt	Added TS6504 error for exports with node20
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=nodenext).errors.txt.diff	Removed diff for nodenext module
testdata/baselines/reference/submodule/conformance/nodeModulesExportsBlocksTypesVersions(module=nodenext).errors.txt	Added TS6504 error for exports with nodenext
testdata/baselines/reference/submodule/conformance/resolvesWithoutExportsDiagnostic1(moduleresolution=bundler).errors.txt.diff	Removed diff for bundler module resolution
testdata/baselines/reference/submodule/conformance/resolvesWithoutExportsDiagnostic1(moduleresolution=bundler).errors.txt	Added TS6504 errors for multiple JS files with bundler resolution
testdata/baselines/reference/submodule/conformance/resolvesWithoutExportsDiagnostic1(moduleresolution=node16).errors.txt.diff	Removed diff for node16 module resolution
testdata/baselines/reference/submodule/conformance/resolvesWithoutExportsDiagnostic1(moduleresolution=node16).errors.txt	Added TS6504 errors for multiple JS files with node16 resolution

internal/compiler/fileloader.go

Co-authored-by: Copilot <[email protected]>

sheetalkamat · 2025-11-20T22:15:18Z

internal/compiler/fileloader.go

 			libFile:            libFile,
 			includeReason:      includeReason,
 		})
+	} else if tspath.HasExtension(absPath) {


This seems good but arent we currently checking this only for root files and not for resolved triple slash refs or project reference files etc as we did in strada

That's true; Strada had this in getSourceFileFromReferenceWorker which was called by processSourceFile, so we do need to do this somewhere else.

Let's try this for fun:

@copilot Reexamine program.ts's getSourceFileFromReferenceWorker, callers, etc, and place this check in the correct place in the Go implementation.

Fixed in b07f77b. Moved the check from addRootTask to parseTask.load() so it now catches all file inclusion paths (root files, triple-slash references, type references, etc.). Tested with both tsgo a.js and triple-slash references like /// <reference path="helper.js" /> - both now correctly report TS6504.

Moved the TS6504 diagnostic check from addRootTask to parseTask.load() so it catches all file inclusion paths (root files, triple-slash refs, type refs, etc.), not just root files. This matches TypeScript's behavior where getSourceFileFromReferenceWorker is called for all file types. Co-authored-by: jakebailey <[email protected]>

jakebailey · 2025-12-02T00:44:29Z

This introduces a data race.

I'm not sure includeProcessor is safe for concurrent use, yet the existing code appears to already touch this concurrently and I think that is likely wrong.

This PR has:

==================
WARNING: DATA RACE
Read at 0x00c016a3e8c8 by goroutine 113684:
  github.com/microsoft/typescript-go/internal/compiler.(*includeProcessor).addProcessingDiagnostic()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/includeprocessor.go:59 +0x998
  github.com/microsoft/typescript-go/internal/compiler.(*parseTask).load()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:89 +0x8b8
  github.com/microsoft/typescript-go/internal/compiler.(*filesParser).start.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:253 +0x344
  github.com/microsoft/typescript-go/internal/core.(*parallelWorkGroup).Queue.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/core/workgroup.go:40 +0x2e
  sync.(*WaitGroup).Go.func1()
      /home/cloudtest/.local/share/microsoft-go/go1.25.4-2/src/sync/waitgroup.go:239 +0x5d

Previous write at 0x00c016a3e8c8 by goroutine 113687:
  github.com/microsoft/typescript-go/internal/compiler.(*includeProcessor).addProcessingDiagnostic()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/includeprocessor.go:59 +0xa31
  github.com/microsoft/typescript-go/internal/compiler.(*parseTask).load()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:89 +0x8b8
  github.com/microsoft/typescript-go/internal/compiler.(*filesParser).start.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/compiler/filesparser.go:253 +0x344
  github.com/microsoft/typescript-go/internal/core.(*parallelWorkGroup).Queue.func1()
      /mnt/vss/_work/typescript-go/typescript-go/internal/core/workgroup.go:40 +0x2e
  sync.(*WaitGroup).Go.func1()
      /home/cloudtest/.local/share/microsoft-go/go1.25.4-2/src/sync/waitgroup.go:239 +0x5d

Which is obviously wrong, but resolveTypeReferenceDirectives today already does p.includeProcessor.addProcessingDiagnostic without any locking, so I think we have just been lucky and not hit this.

jakebailey · 2025-12-03T03:16:56Z

@copilot See above. Make includeProcessor safe for concurrent use.

Added mutex protection to includeProcessor to prevent concurrent access to processingDiagnostics and fileIncludeReasons. The data race was occurring when multiple goroutines in parseTask.load() called addProcessingDiagnostic simultaneously. Changes: - Added mu sync.Mutex to includeProcessor struct - Protected all reads/writes to processingDiagnostics with mutex - Protected all reads/writes to fileIncludeReasons with mutex - Added addFileIncludeReason() and getFileIncludeReasons() helper methods - Updated all direct field accesses to use protected methods - Used maps.Copy in GetIncludeReasons() as suggested by linter Co-authored-by: jakebailey <[email protected]>

Copilot · 2025-12-03T03:38:46Z

@copilot See above. Make includeProcessor safe for concurrent use.

Fixed in d1c8f3b. Added mutex protection to includeProcessor to prevent concurrent access to processingDiagnostics and fileIncludeReasons. All reads and writes to these fields are now protected by the mutex, and helper methods addFileIncludeReason() and getFileIncludeReasons() were added for safe concurrent access. Tests pass with race detector enabled.

Initial plan

3978c47

Copilot AI assigned Copilot and jakebailey Nov 15, 2025

Copilot started work on behalf of jakebailey November 15, 2025 19:22 View session

Add missing TS6504 error for JS files without allowJs

4a047d6

Co-authored-by: jakebailey <[email protected]>

Copilot AI changed the title ~~[WIP] Fix missing error when JS file compiled without allowJs~~ Add missing TS6504 diagnostic for JavaScript files without allowJs Nov 15, 2025

Copilot AI requested a review from jakebailey November 15, 2025 19:48

Copilot finished work on behalf of jakebailey November 15, 2025 19:48

jakebailey marked this pull request as ready for review November 19, 2025 20:52

jakebailey requested review from RyanCavanaugh, Copilot and sheetalkamat November 19, 2025 20:52

jakebailey approved these changes Nov 19, 2025

View reviewed changes

Copilot started reviewing on behalf of jakebailey November 19, 2025 20:52 View session

Copilot finished reviewing on behalf of jakebailey November 19, 2025 20:56

Copilot AI reviewed Nov 19, 2025

View reviewed changes

internal/compiler/fileloader.go Outdated Show resolved Hide resolved

Apply suggestion from @Copilot

ae11735

Co-authored-by: Copilot <[email protected]>

sheetalkamat reviewed Nov 20, 2025

View reviewed changes

Copilot started work on behalf of jakebailey November 20, 2025 22:23 View session

Copilot AI requested a review from jakebailey November 20, 2025 22:55

Copilot finished work on behalf of jakebailey November 20, 2025 22:55

Merge branch 'main' into copilot/fix-missing-error-when-js-file

8bae50b

Copilot started work on behalf of jakebailey December 3, 2025 03:17 View session

Copilot finished work on behalf of jakebailey December 3, 2025 03:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add missing TS6504 diagnostic for JavaScript files without allowJs #2104

Add missing TS6504 diagnostic for JavaScript files without allowJs #2104

Copilot AI commented Nov 15, 2025 •

edited

Loading

Uh oh!

jakebailey left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

sheetalkamat Nov 20, 2025

Uh oh!

jakebailey Nov 20, 2025

Uh oh!

Copilot AI Nov 20, 2025

Uh oh!

jakebailey commented Dec 2, 2025 •

edited

Loading

Uh oh!

jakebailey commented Dec 3, 2025

Uh oh!

Copilot AI commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add missing TS6504 diagnostic for JavaScript files without allowJs #2104

Are you sure you want to change the base?

Add missing TS6504 diagnostic for JavaScript files without allowJs #2104

Conversation

Copilot AI commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Uh oh!

sheetalkamat Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakebailey commented Dec 3, 2025

Uh oh!

Copilot AI commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Nov 15, 2025 •

edited

Loading

jakebailey commented Dec 2, 2025 •

edited

Loading