fix(toTypeReExports): dedupe duplicate type re-exports for class/enum declarations#548
fix(toTypeReExports): dedupe duplicate type re-exports for class/enum declarations#548greymoth-jp wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthrough
Type Re-export Deduplication
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/dts.test.ts (1)
150-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd enum and const-enum cases to this regression.
This pins the class path, but the stated contract also covers
enumandconst enum. Extending this into a small table-driven test would keep those two variants from drifting untested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/dts.test.ts` around lines 150 - 183, The regression test currently covers only the class re-export path in generateTypeDeclarations, but the dedupe behavior is also expected for enum and const enum declarations. Update the existing test in test/dts.test.ts (the createUnimport / generateTypeDeclarations case) to include enum and const enum variants, preferably in a small table-driven form, so all three declarationType values are asserted to produce a single re-export and avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/dts.test.ts`:
- Around line 150-183: The regression test currently covers only the class
re-export path in generateTypeDeclarations, but the dedupe behavior is also
expected for enum and const enum declarations. Update the existing test in
test/dts.test.ts (the createUnimport / generateTypeDeclarations case) to include
enum and const enum variants, preferably in a small table-driven form, so all
three declarationType values are asserted to produce a single re-export and
avoid future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ec4f468-1880-451e-9203-777ee11d5b5a
📒 Files selected for processing (2)
src/utils.tstest/dts.test.ts
toTypeReExportsmaps every type import in a module to anexport type { ... }specifier without collapsing duplicates.dedupeImportskeeps class, enum and const enum imports un-collapsed so a value and a type entry of the same declaration can coexist, which means the same type name can reach this step more than once. When it does, the generated re-export lists it twice:export type { Foo, Foo }is a duplicate identifier (TS2300). It is currently suppressed because the line sits under the existing// @ts-ignore, but the output is still malformed.toExportsgot the same dedupe in #529; this brings the type re-export in line.The change collapses entries by their re-exported name (
as ?? name) before joining, so distinct names and aliases are untouched. The added test reproduces the duplicate: reverting only the source change makes it fail withexport type { Foo, Foo }, and the full suite stays green.Summary by CodeRabbit