feat: support cross-module pure detection in inner graph#20907
Merged
Conversation
🦋 Changeset detectedLatest commit: 3f1d246 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is packaged and the instant preview is available (c95b845). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@c95b845
yarn add -D webpack@https://pkg.pr.new/webpack@c95b845
pnpm add -D webpack@https://pkg.pr.new/webpack@c95b845 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #20907 +/- ##
==========================================
+ Coverage 91.59% 91.62% +0.03%
==========================================
Files 573 573
Lines 59541 59664 +123
Branches 16077 16119 +42
==========================================
+ Hits 54534 54668 +134
+ Misses 5007 4996 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9238fbc to
8c90e76
Compare
e7c4f69 to
bb09288
Compare
5e7593a to
4f89ebf
Compare
| commentsStartPos = /** @type {Range} */ (arg.range)[1]; | ||
| return pureFlag; | ||
| }); | ||
| } |
Member
There was a problem hiding this comment.
Why we remove it from here? We use isPure in other places, so it can brings some regressions in optimizations
Member
Author
There was a problem hiding this comment.
Good catch. We ran into this problem before and tried moving it here as a workaround, but it's now properly fixed via pureConditionByCallExpr, so let's revert it back. Thanks for raising it.
Member
|
@hai-x Looks good, only one question |
Member
|
@hai-x can you take a look at copilot review (should we add a test case for such usage), also can you rebase, thanks, I think it is time to prepare a new minor release |
85923e0 to
bd992e9
Compare
bd992e9 to
bae60c5
Compare
| usedExportsOfPureSource | ||
| } from "./pure-source"; | ||
|
|
||
| const pure1 = pureExport1(1); |
| } from "./pure-source"; | ||
|
|
||
| const pure1 = pureExport1(1); | ||
| const pure2 = pureExport2(1); |
|
|
||
| const pure1 = pureExport1(1); | ||
| const pure2 = pureExport2(1); | ||
| const pure3 = pureExport3(1); |
Member
Author
|
@alexander-akait It raise some issues about unused variable. But for now we should keep the variable declarations as-is, because the inner graph currently collects top-level function, class, and variable declarations and doesn't detect top-level But I think we can support bare expression statements in a follow-up PR. |
Member
|
Got it, let's improve |
827a1f1 to
6bfb5ca
Compare
6bfb5ca to
3f1d246
Compare
Contributor
Types CoverageCoverage after merging refactor-innergraph into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @@ -0,0 +1,16 @@ | |||
| import * as ns from "./pure-source"; | |||
|
|
|||
| const pure1 = ns.pureExport1(1); | |||
| import * as ns from "./pure-source"; | ||
|
|
||
| const pure1 = ns.pureExport1(1); | ||
| const pure2 = ns.pureExport2(1); |
|
|
||
| const pure1 = ns.pureExport1(1); | ||
| const pure2 = ns.pureExport2(1); | ||
| const pure3 = ns.pureExport3(1); |
| const pure1 = ns.pureExport1(1); | ||
| const pure2 = ns.pureExport2(1); | ||
| const pure3 = ns.pureExport3(1); | ||
| const pure4 = ns.pureExport4(1); |
| const pure2 = ns.pureExport2(1); | ||
| const pure3 = ns.pureExport3(1); | ||
| const pure4 = ns.pureExport4(1); | ||
| const pure5 = ns.pureExport5(1); |
| const pure3 = ns.pureExport3(1); | ||
| const pure4 = ns.pureExport4(1); | ||
| const pure5 = ns.pureExport5(1); | ||
| const pureDefault = ns.default(1); |
| const pure5 = ns.pureExport5(1); | ||
| const pureDefault = ns.default(1); | ||
|
|
||
| const impure = ns.impureExport(2); |
alexander-akait
approved these changes
May 25, 2026
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
2 participants
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.
Summary
Refactors the InnerGraph tree-shaking utility so purity can be tracked across module boundaries, not just
within a single module.
What kind of change does this PR introduce?
TL;DR
A
/*#__NO_SIDE_EFFECTS__*/annotation in one module is now honored by call sites in other modules — notjust within the file it was declared in.
Example
Before this PR,
pureFnshows up inusedExportsbecause the/*#__NO_SIDE_EFFECTS__*/hint stayed insidepure.jsand never reached the importer, so the call sitepureFn(1)was treated as a usage withsideEffects.After this PR,
pureFnexport can be tree-shaken.How the optimization works:
Producer records purity locally. While parsing
pure.js, the parser sees the/*#__NO_SIDE_EFFECTS__*/and writes the name intomodule.buildInfo.pureFunctions. At this point the purity is just a per-module fact.Purity is lifted onto the export descriptor. When
HarmonyExportSpecifierDependency / HarmonyExportExpressionDependencybuild theirExportsSpec, they consultbuildInfo.pureFunctionsand emit{ name: "pureFn", isPure: true }. This is theonly channel that crosses the module boundary.
FlagDependencyExportsPluginpropagatesisPureto ExportInfo and thenew
isPureflag is copied onto the matchingExportInfo.Importer tags the local binding with a deferred pure predicate. When
index.jsis parsed, the inner graphcreates a
TopLevelSymbolfor the importedpureFnand itspurefield is a function for purpose ofdeferringinstead of definitetrue. The deferral matters because the importing module may not yet be parsed.Per-compilation state.
Inner-graphstate is now scoped to the Compilation(
getInnerGraph(compilation)) and keyed byModule, which is what lets step 4 reach into another module'sExportsInfofrom inside the importer's graph.Inference resolves the call. When
inferDependencyUsagewalks the importer's graph, it reaches thepureFn(1)call, asks the symbolisPure(compilation, module), gets true, so propagate pure-symbol usage through the inner graph.Did you add tests for your changes?
Yes
Does this PR introduce a breaking change?
No
If relevant, what needs to be documented once your changes are merged or what have you already documented?
No
Use of AI
Partial