Analysis

The refactored TreeShakingMode::ReexportsOnly logic in lines 177-216 changes the conditional structure in a way that breaks tree shaking for certain modules.

Before the change: When part is ModulePart::Export(_), the code would:

• If split_locals_and_reexports() returns true → apply tree shaking with facade module
• If split_locals_and_reexports() returns false → still apply tree shaking directly on the module via apply_reexport_tree_shaking(Vc::upcast(*module), part, side_effect_free_packages)

After the change: The logic now checks split_locals_and_reexports() first:

• If true → apply appropriate tree shaking based on part type
• If false → always return Vc::upcast(*module) unchanged (line 215)

This means modules that don't have reexports (where split_locals_and_reexports() returns false) will completely bypass tree shaking even when explicitly requested for export parts in ReexportsOnly mode. This can result in larger bundles and dead code inclusion, as the tree shaking that should occur for these modules is never applied.

The split_locals_and_reexports() method returns true only when a module has star exports or imported bindings that are re-exported, so regular modules with only local exports will be affected by this bug.

Recommendation

Restore the original conditional structure where apply_reexport_tree_shaking is called for ModulePart::Export(_) regardless of whether the module splits locals and reexports. The logic should be:

if let Some(part) = part {
    match part {
        ModulePart::Evaluation => {
            if *module.get_exports().split_locals_and_reexports().await? {
                Vc::upcast(EcmascriptModuleLocalsModule::new(*module))
            } else {
                Vc::upcast(*module)
            }
        }
        ModulePart::Export(_) => {
            let side_effect_free_packages = module_asset_context
                .side_effect_free_packages()
                .resolve()
                .await?;

            if *module.get_exports().split_locals_and_reexports().await? {
                apply_reexport_tree_shaking(
                    Vc::upcast(EcmascriptModuleFacadeModule::new(
                        Vc::upcast(*module),
                        ModulePart::facade(),
                    ).resolve().await?),
                    part,
                    side_effect_free_packages,
                )
            } else {
                apply_reexport_tree_shaking(
                    Vc::upcast(*module),
                    part,
                    side_effect_free_packages,
                )
            }
        }
        _ => bail!("Invalid module part...")
    }
} else if *module.get_exports().split_locals_and_reexports().await? {
    Vc::upcast(EcmascriptModuleFacadeModule::new(
        Vc::upcast(*module),
        ModulePart::facade(),
    ))
} else {
    Vc::upcast(*module)
}

Analysis

In the Dynamic match arm of follow_reexports, the code uses [] => unreachable!(), when handling dynamic_exporting_modules. While the current logic in find_export_from_reexports should prevent returning a Dynamic variant with an empty vector (it returns NotFound when dynamic_exporting_modules.is_empty() is true), using unreachable!() is risky in production code.

If there's any bug in the logic, race condition, or unexpected edge case that results in an empty vector being passed to the Dynamic case, the application will panic rather than handling the error gracefully. The previous implementation handled this case by returning a NotFound result, which was more defensive programming.

Recommendation

Replace the unreachable!() with graceful error handling that matches the old behavior. Instead of [] => unreachable!(),, use:

[] => Ok(FollowExportsResult {
    module,
    export_name: Some(export_name),
    ty: FoundExportType::NotFound,
}.cell()),

This maintains the same behavior as the previous implementation while avoiding potential panics.