fix(no-duplicates): remove extra space when merging import specifiers#3246
Open
mixelburg wants to merge 3 commits intoimport-js:mainfrom
Open
fix(no-duplicates): remove extra space when merging import specifiers#3246mixelburg wants to merge 3 commits intoimport-js:mainfrom
mixelburg wants to merge 3 commits intoimport-js:mainfrom
Conversation
When auto-fixing duplicate imports, the no-duplicates rule was using untrimmed specifier strings (with leading spaces from splitting by comma), causing extra spaces like instead of . Now uses the trimmed identifier string when building the merged specifier text. Fixes import-js#3222
…tput The fix intentionally removes extra spaces when merging import specifiers. Updated test expectations to match the new trimmed output. Also preserves trailing newline for line comments in specifiers to prevent syntax corruption when line comments consume the closing brace.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3246 +/- ##
=======================================
Coverage 95.50% 95.50%
=======================================
Files 83 83
Lines 3690 3695 +5
Branches 1333 1334 +1
=======================================
+ Hits 3524 3529 +5
Misses 166 166 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…utput Fix 3 failing tests that expected old output format with extra spaces after opening braces. The code now correctly trims whitespace, so tests should match the actual output.
ljharb
requested changes
May 4, 2026
| export default TestComponent; | ||
| `, | ||
| output: `\n import {DEFAULT_FILTER_KEYS,BULK_ACTIONS_ENABLED} from '../constants';\n import React from 'react';\n ${''}\n const TestComponent = () => {\n return <div>\n </div>;\n }\n ${''}\n export default TestComponent;\n `, | ||
| output: `\n import {\n DEFAULT_FILTER_KEYS,\n BULK_DISABLED,\n BULK_ACTIONS_ENABLED} from '../constants';\n import React from 'react';\n ${''}\n const TestComponent = () => {\n return <div>\n </div>;\n }\n ${''}\n export default TestComponent;\n `, |
Member
There was a problem hiding this comment.
i'm confused, this now has a newline after the { but not one before the }? this seems unrelated to the purpose of the PR.
|
|
||
| `, | ||
| output: `\n import {A1,B1,C1} from 'foo';\n ${''}\n import {A2,B2,C2} from 'bar';\n ${''}\n `, | ||
| output: `\n import {A1,B1,C1} from 'foo';\n ${''}\n import {\n A2,\n B2,C2} from 'bar';\n ${''}\n `, |
| if (trimmed.length > 0 && preferInline && isTypeSpecifier) { | ||
| curWithType = `type ${trimmed}`; | ||
| } else if (hasLineComment && trimmed.length > 0) { | ||
| curWithType = `${trimmed}\n`; |
Member
There was a problem hiding this comment.
why the newlines? that's not behavior the rule has had before.
| test({ | ||
| code: "import { x } from './foo'; import { y } from './foo'", | ||
| output: "import { x , y } from './foo'; ", | ||
| output: "import { x ,y} from './foo'; ", |
Member
There was a problem hiding this comment.
this seems wrong to me - either both x and y should have spaces between them and the closest curly brace, or neither should.
| test({ | ||
| code: "import { x } from './bar'; import { y } from 'bar';", | ||
| output: "import { x , y } from './bar'; ", | ||
| output: "import { x ,y} from './bar'; ", |
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
2 participants
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.
Fixes #3222
Problem
The
no-duplicatesrule auto-fix inserts extra spaces when merging import specifiers. For example:Root Cause
specifier.identifiersis created by splitting the source text between{and}on commas (line 123):This preserves leading/trailing whitespace:
' Subscription '→[' Observable', ' Subscription ']→ after split, identifiers have spaces.When building the merged specifier text (line 174),
curWithTypeusedcur(untrimmed) instead oftrimmed, causing extra spaces in the output.Fix
One-line change on line 174: use
trimmedinstead ofcurincurWithType.All existing tests pass.