iframe-proxy

8ctavio · 2026-04-14T04:36:22Z

Resolves #524.

The current implementation of dedupeImports ignores negative priorities in some cases. This PR implements proper support for imports' negative priorities.

In the linked issue, there's a warning for Nuxt's useAppConfig import, which uses a negative priority of -1, so I think not properly handling negative priorities is causing that issue.

Summary by CodeRabbit

Bug Fixes
- Refined duplicate-import resolution so negative and zero priorities are handled correctly; default-priority imports are no longer erroneously replaced by lower-priority duplicates.
Tests
- Added tests to verify behavior for negative and zero priority duplicates and ensure no spurious warnings are emitted.

coderabbitai · 2026-04-14T04:36:37Z

📝 Walkthrough

Walkthrough

The duplicate-import resolution in dedupeImports was adjusted to compute previous-priority by considering complement entries via Math.max and to use nullish coalescing (?? 1) when deriving priority values, affecting how duplicates with negative or zero priorities are resolved.

Changes

Cohort / File(s)	Summary
Duplicate Import Resolution Logic `src/utils.ts`	Replaced `
Deduplication Test Coverage `test/dedupe.test.ts`	Added tests asserting that duplicate imports with `priority: -1` and `priority: 0` do not override default-priority entries and that no warning message is emitted.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

feat(dedupeImports): preserve imports with same from and name, but different type #507: Prior rewrite of dedupeImports that introduced complement-aware priority handling; changes here refine priority comparisons and edge-case behavior.

Poem

🐰 A hop, a check, a priority dance,
Complement weighed, no duplicate chance.
Zero and minus bow out with grace,
One import stays in its rightful place. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR fixes negative priority handling in dedupeImports by using nullish coalescing instead of logical-or operators, which allows priorities like -1 and 0 to be properly considered. Test cases verify that negative and zero priorities are correctly treated as lower precedence than defaults, directly addressing issue `#524`'s duplicate warning problem.
Out of Scope Changes check	✅ Passed	All changes are within scope: modifications to dedupeImports priority comparisons in src/utils.ts and corresponding test coverage in test/dedupe.test.ts directly address the linked issue about negative priority handling.
Title check	✅ Passed	The title clearly and specifically identifies the main fix: handling negative and zero import priorities in dedupeImports, which directly addresses the changeset and the linked issue `#524`.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-04-14T04:37:32Z

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.60%. Comparing base (a4a5e0a) to head (94bce12).
⚠️ Report is 162 commits behind head on main.

Files with missing lines	Patch %	Lines
src/utils.ts	66.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
- Coverage   98.73%   94.60%   -4.13%     
==========================================
  Files          14       13       -1     
  Lines        1817     1001     -816     
  Branches      374      332      -42     
==========================================
- Hits         1794      947     -847     
- Misses         23       50      +27     
- Partials        0        4       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

🧹 Nitpick comments (1)

test/dedupe.test.ts (1)

70-93: Consider asserting no warning for the negative-priority case.

This test verifies winner selection, but not warning suppression. Adding an empty-warning assertion would harden the regression coverage.

Suggested test addition

   it('should respect negative priority', () => {
     expect(dedupeImports(
       [
         {
           name: 'foo',
           from: 'module1',
         },
         {
           name: 'foo',
           from: 'module2',
           priority: -1,
         },
       ],
       warnFn,
     )).toMatchInlineSnapshot(`
       [
         {
           "from": "module1",
           "name": "foo",
         },
       ]
     `)
+    expect(warnMsg).toMatchInlineSnapshot('""')
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/dedupe.test.ts` around lines 70 - 93, The test for negative priority
should also assert that no warning was emitted: after calling dedupeImports in
the 'should respect negative priority' test, add an assertion that warnFn was
not called (e.g., expect(warnFn).not.toHaveBeenCalled() or
expect(warnFn.mock.calls.length).toBe(0)) so the test verifies both winner
selection and warning suppression for the negative-priority case; locate the
call to dedupeImports and the warnFn mock in this test to place the assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/dedupe.test.ts`:
- Around line 70-93: The test for negative priority should also assert that no
warning was emitted: after calling dedupeImports in the 'should respect negative
priority' test, add an assertion that warnFn was not called (e.g.,
expect(warnFn).not.toHaveBeenCalled() or
expect(warnFn.mock.calls.length).toBe(0)) so the test verifies both winner
selection and warning suppression for the negative-priority case; locate the
call to dedupeImports and the warnFn mock in this test to place the assertion.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7eb1b1b2-0ccc-43e6-b5ac-369b4f2977fd

📥 Commits

Reviewing files that changed from the base of the PR and between 10a9ef3 and 2164c86.

📒 Files selected for processing (2)

src/utils.ts
test/dedupe.test.ts

danielroe · 2026-04-21T16:12:07Z

    const prevImpComplement = deduped.get(altName)
-    const priorityDiff = (currImp.priority || 1) - Math.max(prevImp.priority || 1, prevImpComplement?.priority || 1)
+    const prevPriority = prevImpComplement
+      ? Math.max(prevImp.priority || 1, prevImpComplement.priority || 1)


we should use ?? rather than || to respect priority of 0

Should we do that now? I kept || for compatibility, but I can change to ?? if that's not an issue.

8ctavio added 2 commits April 13, 2026 22:13

fix(dedupeImports): support negative import priorities

ae882f2

test(dedupeImports): consider imports' negative priorities

2164c86

coderabbitai Bot reviewed Apr 14, 2026

View reviewed changes

danielroe reviewed Apr 21, 2026

View reviewed changes

fix: apply @danielroe's fix

94bce12

antfu changed the title ~~fix(dedupeImports): support negative import priorities~~ fix(dedupeImports): respect negative and zero import priorities Apr 22, 2026

antfu merged commit 4f462cb into unjs:main Apr 22, 2026
4 of 6 checks passed

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dedupeImports): respect negative and zero import priorities#527

fix(dedupeImports): respect negative and zero import priorities#527
antfu merged 3 commits intounjs:mainfrom
8ctavio:fix/dedupe-negative-priority

8ctavio commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

danielroe Apr 21, 2026 •

edited

Loading

Uh oh!

8ctavio Apr 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

8ctavio commented Apr 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

danielroe Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

8ctavio Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

8ctavio commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

codecov Bot commented Apr 14, 2026 •

edited

Loading

danielroe Apr 21, 2026 •

edited

Loading