Fix assertion violation in auto-imports for JSX tag when React is type-imported#3650
Fix assertion violation in auto-imports for JSX tag when React is type-imported#3650
Conversation
…SX tags When a JSX tag triggers the "cannot be used as a value because it was imported using import type" error, getSymbolNamesToImport may return multiple names (the component name AND the JSX namespace like React). Instead of panicking when there are multiple names, iterate over all names attempting type-only promotion for each, and fall back to regular auto-import logic for names where promotion is not applicable. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/d390d5fd-644f-4031-8a82-ca0441ed46cb Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
|
Are each of these discrete fixes? Can they not be combined? Aside, I noticed that |
andrewbranch
left a comment
There was a problem hiding this comment.
Huh yeah let's actually fix the behavior instead of doing something weird that doesn't panic. The question is, why is there more than one symbol name here?
|
@copilot ^ |
…import fallback For the type-only promotion error, iterate over symbol names from getSymbolNamesToImport and return the first valid promotion fix. Other missing symbols (like unresolved JSX component names) have their own separate error diagnostics that trigger auto-import independently. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/f28dae04-ed90-4058-926b-6ad8d3234cec Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
Multiple symbol names come from Simplified in 8134793 to just iterate over the names and return the first valid promotion fix. No auto-import fallback — other missing symbols get their own error diagnostics.
Simplified in 8134793 — now only returns the single promotion fix (no more separate fixes to combine). The auto-import for the missing component comes from its own "Cannot find name" error diagnostic, not from this code path. Re |
| `import type React from "./react"; | ||
| import React from "./react"; |
There was a problem hiding this comment.
This should not be offered as a fix. This code is silly. I understand it may be part of a different code path but we may as well make this whole test behave like it should.
There was a problem hiding this comment.
Fixed in cfe5f6c. The silly import type React + import React fix is no longer offered. Changed needsJsxNamespaceFix to return false for type-only imports, since the promotion path handles them separately. The test now expects only the single promotion fix.
|
@copilot ^ |
- Type-only promotion path: directly determine JSX candidates (token text + JSX namespace) instead of using getSymbolNamesToImport, which has auto-import filtering logic that doesn't apply - needsJsxNamespaceFix: return false for type-only imports since the promotion path handles them separately, preventing silly duplicate imports - getFixesInfoForNonUMDImport: set isJsxNamespaceFix to match Strada - Add edge case test where both Foo and React are type-only imported Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/a4d3e15d-7971-430e-99c5-81a0ae184f30 Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
|
@copilot your inline checks look less sophisticated than
From there, if both symbols resolve to a type-only import, we can easily choose between just providing both fixes, resulting in duplicates like the current test shows (not the end of the world, better than crashing or omitting a fix) or correlating the symbol name to the error message, whether through string matching or add-on data later. |
…flag Modify getSymbolNamesToImport to return symbolNameInfo structs with name and isTypeOnly fields. This centralizes the JSX checking logic and allows callers to properly filter: - Type-only promotion path: filters to isTypeOnly symbols only - Auto-import path: skips isTypeOnly symbols (handled by promotion) Also reverts needsJsxNamespaceFix to match Strada behavior (includes type-only imports in the candidate list) and includes component name when it resolves as type-only (for the promotion path to use). Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/d5749c27-5468-436b-adb5-cfe32e9f8e35 Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
Refactored in 32a7b79. |
Use the diagnostic message to correlate which symbol each type-only promotion error is about. The message format is "'X' cannot be used as a value because it was imported using 'import type'." so we check for the symbol name in single quotes. This eliminates duplicate fixes when both component and JSX namespace are type-only imported. Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/eb5910bf-d44c-483e-bdb5-5b6b250eb6e7 Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a crash in the JSX auto-import/type-only-promotion code path when a JSX tag triggers the “cannot be used as a value because it was imported using import type” diagnostic and multiple symbol names are involved (e.g., component + JSX namespace).
Changes:
- Refactors
getSymbolNamesToImportto return{name, isTypeOnly}data so callers can correctly filter type-only vs value imports. - Updates the type-only promotion path to handle multiple candidate names and avoids generating duplicate auto-imports for type-only symbols.
- Adds fourslash tests to reproduce the original crash and validate the multi-type-only disambiguation behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/ls/codeactions_importfixes.go | Removes the hard panic by supporting multiple symbol names and adds type-only-aware filtering/disambiguation. |
| internal/fourslash/tests/codeFixPromoteTypeOnlyImportJsxTag_test.go | Adds regression and edge-case tests covering JSX + type-only import promotion. |
Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/1c2ae9fb-44fd-48a5-b940-a39404eb47f0 Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
When a JSX tag triggers the
_0_cannot_be_used_as_a_value_because_it_was_imported_using_import_typeerror,getSymbolNamesToImportcan return multiple names (e.g., both the component name and the JSX namespace). The code had a hardpanicrequiring exactly one name.This happens because
getSymbolNamesToImportis a general-purpose function that returns all names needed for a JSX tag (e.g.,["Foo", "React"]), but for the type-only promotion error, only the type-imported symbol (React) needs promotion. The component name (Foo) isn't imported at all — it has its own separate "Cannot find name" error diagnostic.getSymbolNamesToImportto return richersymbolNameInfostructs containing the symbol name and anisTypeOnlyflag (determined viaGetTypeOnlyAliasDeclaration). This centralizes the resolution logic and allows callers to properly filter:isTypeOnlysymbols and generates promotion fixes for each.isTypeOnlysymbols (handled by promotion), preventing sillyimport type React+import Reactduplicates.'<symbolName>'in single quotes) to correlate each diagnostic to its symbol, eliminating duplicate fixes.needsJsxNamespaceFixreverted to Strada behavior: Now correctly includes type-only imports in the candidate list (the filtering happens at the call site viaisTypeOnly).getSymbolNamesToImportnow includes the component name when it resolves as type-only (not just when unresolved), so the promotion path can generate fixes for it.isJsxNamespaceFixset:getFixesInfoForNonUMDImportnow setsisJsxNamespaceFixto match Strada (symbolName != symbolToken.Text()).