Skip to content

feat: replace fix with suggestions in no-duplicate-imports#445

Open
Tanujkanti4441 wants to merge 3 commits into
eslint:mainfrom
Tanujkanti4441:remove-fix
Open

feat: replace fix with suggestions in no-duplicate-imports#445
Tanujkanti4441 wants to merge 3 commits into
eslint:mainfrom
Tanujkanti4441:remove-fix

Conversation

@Tanujkanti4441
Copy link
Copy Markdown
Contributor

@Tanujkanti4441 Tanujkanti4441 commented May 7, 2026

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What is the purpose of this pull request?

To remove the fix for duplicate imports that have conditions and apply suggestions to them.

What changes did you make? (Give an overview)

Applied suggestions to the duplicate imports instead of fix.

@import 'test.css' print;
@import 'test.css';

Now, for this code rule will propose two suggestions to remove anyone the duplicates with following messages:
For first one: Remove duplicate @import rule with condition(s) - print.
For second one: Remove duplicate @import rule without conditions.

@import 'test.css' print;
@import 'test.css' print, screen;

Same for this code:
For first one: Remove duplicate @import rule with condition(s) - print.
For second one: Remove duplicate @import rule with condition(s) - print, screen.

@import 'test.css' print;
@import 'test.css' print;

If both duplicates have the same conditions, then the rule will apply autofix and will remove the second one. For now, the autofix only works if the conditions in both then imports are in the same order.

Should we apply the autofix if imports have same conditions but in different order as well?

Related Issues

fixes #441

Is there anything you'd like reviewers to focus on?

@Tanujkanti4441 Tanujkanti4441 marked this pull request as ready for review May 8, 2026 13:34
const importHasConditions = importNode.prelude.children.length > 1;

if (importHasConditions) {
importNode.prelude.children.slice(1).forEach(condition => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syntax of @import also allows specifying a layer and a supports condition (so not everything after is a condition).
Or should this be handled in a follow-up?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR already includes the layer and supports conditions. I have added tests regarding this also. So, I think we can just update the word condition to something more appropriate? like modifier?

Comment thread src/rules/no-duplicate-imports.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make imports an array now instead of converting it from a set to an array for Array#some/find.

loc: node.loc,
messageId: "duplicateImport",
data: { url },
fix(fixer) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the fixer is the same for the autofix and suggestion, I think we should extract this logic to a function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@DMartens DMartens moved this from Needs Triage to Implementing in Triage May 13, 2026
@DMartens DMartens added the accepted There is consensus among the team that this change meets the criteria for inclusion label May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion feature

Projects

Status: Implementing

Development

Successfully merging this pull request may close these issues.

Rule Change: Avoid fix for duplicate imports with condition

2 participants