-
Notifications
You must be signed in to change notification settings - Fork 29k
Turbopack: refactor side effects optimization #82466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
CodSpeed Performance ReportMerging #82466 will improve performances by 4.52%Comparing Summary
Benchmarks breakdown
|
6441f50
to
8149991
Compare
fc0bb56
to
9e3cf69
Compare
9e3cf69
to
2fb7903
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments:
turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/locals/module.rs (lines 50-50):
The code calls removed helper function ModulePart::locals()
which will cause compilation failure.
View Details
Analysis
The helper function ModulePart::locals()
was removed from the ModulePart
implementation in turbopack-core/src/resolve/mod.rs
, but this call site was not updated. The function previously returned ModulePart::Locals
, so this call should be replaced with the direct enum variant.
This will prevent the code from compiling since the function no longer exists.
Recommendation
Replace ModulePart::locals()
with ModulePart::Locals
.
turbopack/crates/turbopack-ecmascript/src/tree_shake/asset.rs (lines 342-342):
The code calls removed helper function ModulePart::exports()
which will cause compilation failure.
View Details
Analysis
The helper function ModulePart::exports()
was removed from the ModulePart
implementation in turbopack-core/src/resolve/mod.rs
, but this call site was not updated. The function previously returned ModulePart::Exports
, so this call should be replaced with the direct enum variant.
This will prevent the code from compiling since the function no longer exists.
Recommendation
Replace ModulePart::exports()
with ModulePart::Exports
in the part_dep(ModulePart::exports())
call.
turbopack/crates/turbopack-ecmascript/src/side_effect_optimization/facade/module.rs
Show resolved
Hide resolved
8149991
to
8f8c78e
Compare
2fb7903
to
50c0d16
Compare
50c0d16
to
c5ae955
Compare
Tests Passed |
) -> Result<Vc<FindExportFromReexportsResult>> { | ||
// TODO why do we need a special case for this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the discussions in #78047
c5ae955
to
ac71779
Compare
What?
Refactors the side effects optimization.
<module evaluation>
and the<exports>
module.<local>
and the facade.require()
andimport *
will resolve to the facade moduleimport "..."
will resolve to the localsimport { x } from "..."
will follow reexports and finally imports from a locals moduleCloses PACK-5203