Skip to content

fix: remove effect packages from optimizePackageImports list#80761

Closed
fubhy wants to merge 1 commit into
vercel:canaryfrom
fubhy:remove-effect-pkgs
Closed

fix: remove effect packages from optimizePackageImports list#80761
fubhy wants to merge 1 commit into
vercel:canaryfrom
fubhy:remove-effect-pkgs

Conversation

@fubhy

@fubhy fubhy commented Jun 22, 2025

Copy link
Copy Markdown

The feature for optimizeModuleImports seems to originate here

The effect packages were first introduced here.

The original intent seems to have been to optimise performance during development time. But it has questionable side effects outside of this intent.

The way aliases are generated here is problematic for at least two reasons:

a) multiple versions of the same package now have their root entrypoint aliased to the same package without taking proper module resolution into consideration
b) it only targets exact matches on root entrypoints - the module exports of these packages are still resolved with the normal module resolution algorithm

This combination means that we get completely undefined runtime behavior.

This PR removes the effect packages from this list to circumvent this problem as a last resort, but I'd suggest to fix this behavior more sustainably instead(!) as it has the potential to generate incredibly difficult to debug runtime behavior that differs between development & production. It makes it easy to ship application versions into production that are broken in ways that are not surfaced during development.

The good intention to cirumvent the the module traversal and scanning performance impact in case of barrel file imports during development time is noble but it should not have the unintended side effects with these server path alias rewrites as observed.

NOTE: The negative impact of this behavior goes beyond effect and would equally apply to some of the other packages in that list with potentially worse outcomes. Instead of explicitly breaking on load it would cause undefined runtime behavior for most other packages.

@vercel

vercel Bot commented Jun 22, 2025

Copy link
Copy Markdown
Contributor

Notifying the following users due to files changed in this PR based on this repo's notify modifiers:

@timneutkens, @ijjk, @shuding, @huozhi:

packages/next/src/server/config.ts

@ijjk

ijjk commented Jun 22, 2025

Copy link
Copy Markdown
Member

Allow CI Workflow Run

  • approve CI run for commit: c7e35c0

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@fubhy

fubhy commented Jun 22, 2025

Copy link
Copy Markdown
Author

Addressed properly in #80769

@fubhy fubhy closed this Jun 22, 2025
@github-actions github-actions Bot added the locked label Jul 7, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants