fix(plugins): treat dynamic(import().then(m => m.X)) as re-export#263
Merged
BartWaardenburg merged 1 commit intofallow-rs:mainfrom May 3, 2026
Merged
Conversation
The Next.js code-splitting idiom
// Foo.tsx
export function Foo() { /* heavy */ }
// Foo-lazy.tsx
export const Foo = dynamic(
() => import('./Foo').then(m => m.Foo),
{ ssr: false },
);
is semantically a re-export of `Foo`, equivalent to
`export { Foo } from './Foo'`, which fallow already does not flag.
Currently both files appear in `duplicate_exports` for `Foo`.
`find_duplicate_exports` now accepts the resolved module list and
extends `re_export_sources` with dynamic-import edges that act as
re-exports. The classification mirrors the static `export from` shape
exactly: a dynamic import counts as a re-export only when the wrapper
module also exports the same name (`Named("X")` requires the wrapper
to export "X"; `Default` requires the wrapper to have a default
export). `Namespace` and `SideEffect` imports never count as
re-exports.
The dynamic-reexport collection lives in a separate
`collect_dynamic_reexport_sources` helper to keep
`find_duplicate_exports` focused on duplicate detection.
Five unit tests:
- happy paths for `.then(m => m.Foo)` and default-import variants
- wrapper has Named dynamic import but exports a different name
- wrapper has Default dynamic import but no default export
- two unrelated modules with no dynamic link still flagged
The two missing-export tests guard against false negatives where a
module dynamically imports something but does not actually re-export
it.
All 16 existing call sites of `find_duplicate_exports` pass `&[]`
for the new parameter, preserving their semantics. Pass `&[]` to opt
out of dynamic-import re-export detection.
BartWaardenburg
added a commit
that referenced
this pull request
May 3, 2026
Three follow-ups surfaced during pre-ship review of #263 and #265: 1. #263 added five tests for the Next.js dynamic-import re-export fix. Four are regression-strength but `dynamic_import_named_without_matching_export_still_flagged` is coverage-adjacent: the wrapper exports `Bar` and the duplicate detection looks for `Foo` between two unrelated modules, so the wrapper's `matches_export` check is never on the path that decides the test outcome. Removing the Named branch of `matches_export` does not make this test fail. Add a true regression test (`dynamic_import_named_mismatched_with_wrapper_export_still_flagged`) where the wrapper exports `Foo` AND dynamically imports a different name `Bar` from source, so without the matches_export Named branch the wrapper would be registered as a re-export edge and the (source, wrapper) `Foo` duplicate would be silently suppressed. Empirically verified: removing the Named branch makes this test fail and leaves the other four passing. 2. #265 added `Plugin::virtual_package_suffixes()` but only via an explicit `impl Plugin for VitestPlugin` block. The three `define_plugin!` macro variants in `plugins/mod.rs` still listed `virtual_module_prefixes` without a peer entry for the new field, so a future plugin author using the macro would have no path to declare suffixes. Extend all three variants to accept `virtual_package_suffixes:` and add a synthetic smoke test to guard future macro regressions. 3. `.claude/rules/plugins.md` `## Plugin trait extensions` section listed `path_aliases` and `virtual_module_prefixes` but not the new `virtual_package_suffixes`. Add it with a one-line description and a Vitest example.
Collaborator
|
Released in v2.63.0. Thanks @fmguerreiro. |
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.
Problem
The standard Next.js code-splitting idiom produces a false-positive
duplicate-export:```tsx
// Foo.tsx
export function Foo() { /* heavy component */ }
// Foo-lazy.tsx
export const Foo = dynamic(
() => import('./Foo').then(m => m.Foo),
{ ssr: false },
);
```
Both files appear in
duplicate_exportsforFoo, even though the lazy wrapper is semantically a re-export, equivalent toexport { Foo } from './Foo'(which fallow already does not flag). Every Next.js codebase usingnext/dynamicto lazy-load Mapbox, charts, editors, or PDF viewers hits this. Documented pattern: https://nextjs.org/docs/app/api-reference/functions/dynamicFix
find_duplicate_exportsaccepts a&[ResolvedModule]and extendsre_export_sourceswith dynamic-import edges that act as re-exports. The new helpercollect_dynamic_reexport_sources(incrates/core/src/analyze/unused_exports.rs) walks resolved modules and, for eachresolved_dynamic_importsentry targeting anInternalModule, adds the source as a re-export source of the wrapper if the wrapper also exports the same shape:Named("X")Namedexport ofXdynamic(() => import('./Foo').then(m => m.Foo))DefaultDefaultexportdynamic(() => import('./Foo'))Namespace/SideEffectThe wrapper-must-export check is the load-bearing detail. Without it, any module with a dynamic import targeting another module would silently suppress all duplicate-export findings between them, even when the dynamic import is for some unrelated purpose.
Tests
5 unit tests in
analyze::unused_exports::tests:dynamic_import_then_member_not_flagged_as_duplicate(named via.then(m => m.X))dynamic_import_without_then_default_not_flagged_as_duplicate(default-import variant; both modules have a default export)dynamic_import_named_without_matching_export_still_flagged(wrapper hasNamed("Foo")import but exports"Bar"; duplicate must still be flagged)dynamic_import_default_without_default_export_still_flagged(wrapper hasDefaultimport but onlyNamedexports; duplicate must still be flagged)duplicate_without_dynamic_link_still_flagged(regression guard for callers that pass non-emptyresolved_moduleswithout dynamic-import links)The last three guard against false negatives.
API surface
find_duplicate_exportsgains a newresolved_modules: &[ResolvedModule]parameter. The existing call site inanalyze::find_dead_code_fullpasses the resolver output. Existing test call sites pass&[], preserving their semantics. Doc comment notes: pass&[]to opt out of dynamic-import detection; staticexport { X } from '...'re-exports continue to be recognized.Verified on
cargo test --workspace,cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --all -- --checkall clean. Real-world: this fork-pinned binary running in a Turborepo monorepo CI cleared the dynamic-wrapper false positive across multiple components.