Restore symbol names in JSX import action titles - #64588
Open
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Open
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Mateusz Burzyński (Andarist) wants to merge 1 commit into
Conversation
typescript-automation
Bot
added
For Uncommitted Bug
Contributor
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 5
Open (7)
Fix.Editsthat alters the returned description… · NewFix.Editsthat alters the returned description… · NewImport_0_from_1for both “add new” and “add to existing”… · NewFix.Editsthat alters the returned description… · NewImport_0_from_1for both “add new” and “add to existing”… · NewincludeSymbolNameInDescriptionbranching is duplicated across fix kinds. To reduce repetition… · NewincludeSymbolNameInDescriptionbranching is duplicated across fix kinds. To reduce repetition… · New
What changed in this PR
Ports Strada behavior to optionally include the symbol name in auto-import fix descriptions, and re-enables previously skipped JSX import name code-fix tests.
Changes:
- Added an
includeSymbolNameInDescriptionflag toautoimport.Fix.Editsand threaded it through key call sites. - Adjusted import code action generation to detect JSX namespace fixes and include the symbol name in the action title when appropriate.
- Unskipped two failing fourslash JSX import-name code-fix tests.
| File | Description |
|---|---|
| tsc/internal/ls/completions.go | Updates autoimport.Fix.Edits invocations to pass the new symbol-in-description flag (currently false) to preserve existing completion detail behavior. |
| tsc/internal/ls/codeactions_importfixes.go | Adds errorIdentifierText to fixInfo and uses it to decide when to include symbol names in import code action titles (JSX namespace cases). |
| tsc/internal/ls/autoimport/fix.go | Extends Fix.Edits signature and conditionally returns a symbol-containing description string for add/update import fixes. |
| tsc/internal/fourslash/tests/importNameCodeFix_jsx4_test.go | Re-enables the previously skipped JSX fourslash test. |
| tsc/internal/fourslash/tests/importNameCodeFix_jsx6_test.go | Re-enables the previously skipped JSX fourslash test. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| formatOptions lsutil.FormatCodeSettings, | ||
| converters *lsconv.Converters, | ||
| preferences lsutil.UserPreferences, | ||
| includeSymbolNameInDescription bool, |
Comment on lines
+79
to
+81
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } |
Comment on lines
+79
to
82
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } | ||
| return edits, diagnostics.Update_import_from_0.Localize(locale, f.ModuleSpecifier), safe |
Comment on lines
+114
to
+116
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } |
Comment on lines
+114
to
117
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } | ||
| return edits, diagnostics.Add_import_from_0.Localize(locale, f.ModuleSpecifier), safe |
Comment on lines
+79
to
82
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } | ||
| return edits, diagnostics.Update_import_from_0.Localize(locale, f.ModuleSpecifier), safe |
Comment on lines
+114
to
117
| if includeSymbolNameInDescription { | ||
| return edits, diagnostics.Import_0_from_1.Localize(locale, f.Name, f.ModuleSpecifier), safe | ||
| } | ||
| return edits, diagnostics.Add_import_from_0.Localize(locale, f.ModuleSpecifier), safe |
Copilot started reviewing on behalf of
Mateusz Burzyński (Andarist)
October 2, 2026 13:55
View session
Copilot stopped reviewing on behalf of
Mateusz Burzyński (Andarist) due to an error
October 2, 2026 13:55
Member
|
Copilot absolutely hates this 😄 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A pretty close port of the Strada's logic that fixes 2 skipped tests