Improve diagnostic for implicit JSX runtime imports in non-module files - #64445
Shrey Shekhar (sh011) wants to merge 3 commits into
Conversation
When a file uses JSX with implicit runtime imports (such as with
`jsx: preserve` and `jsxImportSource: <package>`) but has no top-level
imports or exports, the file is treated as a global script. In this
case, module resolution for the JSX runtime path fails.
Previously, TypeScript emitted TS2875 ("Make sure you have types for the
appropriate package installed"), which was misleading when types were
installed but the file was simply not treated as a module.
Now, if `!ast.IsExternalModule(file)`, TypeScript emits TS2884, advising
the user to set 'moduleDetection' to 'force' or add an empty 'export {}'
statement.
Fixes microsoft#64438
typescript-automation
Bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
@microsoft-github-policy-service agree |
| } | ||
| errorMessage := diagnostics.This_JSX_tag_requires_the_module_path_0_to_exist_but_none_could_be_found_Make_sure_you_have_types_for_the_appropriate_package_installed | ||
| if !ast.IsExternalModule(file) { | ||
| errorMessage = diagnostics.This_JSX_tag_requires_the_module_path_0_to_exist_but_none_could_be_found_If_this_file_is_not_intended_to_be_a_global_script_set_moduleDetection_to_force_or_add_an_empty_export_statement |
There was a problem hiding this comment.
Why do we need to resolve? Is it because resolution can still succeed if there's an ambient declaration? Add a test for when this occurs in a global file with an ambient.
There was a problem hiding this comment.
Yes. In a global script, module resolution won't pull in packages from node_modules, but resolveExternalModule still checks ambient modules via tryFindAmbientModule. If there’s an ambient declare module '...' in the program, it resolves fine and shouldn't error out.
So, I've added a test case in jsxImportSourceNonModuleAmbient.tsx with an ambient declaration in a global file to make sure that the path stays green.
| "category": "Error", | ||
| "code": 2883 | ||
| }, | ||
| "This JSX tag requires the module path '{0}' to exist, but none could be found. If this file is not intended to be a global script, set 'moduleDetection' to 'force' or add an empty 'export {}' statement.": { |
There was a problem hiding this comment.
I don't think I like this message because the 2 sentences don't seem to have anything to do with each other. Not sure what's better but I guess it depends on the answer to the first question.
There was a problem hiding this comment.
I understand now, the wording "none could be found" makes it sound like the package is missing from disk, so jumping straight to moduleDetection felt out of place. Since the actual issue is that global scripts can't import external modules, I updated the message to:
"This JSX tag requires the module path '{0}' to exist, but external modules cannot be imported in a global script. If this file is not intended to be a global script, set 'moduleDetection' to 'force' or add an empty 'export {}' statement."
Is that fine or do you think this might need any more changes?
…e in global scripts
- Update TS2884 to clarify that external modules cannot be imported in
a global script file, directly connecting the missing module path to
the suggestion for moduleDetection/export {}.
- Add test demonstrating that implicit JSX runtime resolution succeeds
in global scripts when an ambient module declaration is present.
Fixes #64438
Analysis
When compiling a file containing JSX with implicit runtime imports (for example, configured with
--jsx preserveand--jsxImportSource @solidjs/web), TypeScript attempts to resolve the implicit JSX runtime import path (@solidjs/web/jsx-runtimeorreact/jsx-runtime).In projects using tools like SolidJS, Babel, or Vite where
--jsx preserveis standard, files without top-levelimportorexportstatements are treated as global scripts unless"moduleDetection": "force"is enabled:Previously, when resolving implicit imports for global scripts via
getJsxNamespaceContainerForImplicitImportintsc/internal/checker/jsx.go, resolution failure always reported error TS2875:This diagnostic was confusing and unhelpful because the required package and type definitions could already be installed in
node_modules. The actual issue was that global scripts cannot import external modules.Fix
Added diagnostic TS2884:
Differentiated non-module files in JSX checker:
In
getJsxNamespaceContainerForImplicitImport(tsc/internal/checker/jsx.go), we check!ast.IsExternalModule(file)when resolving the implicit external module. When the file is not an external module, we now emit TS2884 instead of TS2875.Added tests & baseline updates:
tsc/testdata/tests/cases/compiler/jsxImportSourceNonModule.tsx..errors.txt,.js,.symbols,.types).commentsOnJSXExpressionsArePreserved(jsx=react-jsx,module=commonjs,moduledetection=legacy)andcommentsOnJSXExpressionsArePreserved(jsx=react-jsxdev,module=commonjs,moduledetection=legacy).