Turbopack: refactor side effects optimization (#82466)

### What?

Refactors the side effects optimization.

* Gets rid of the `<module evaluation>` and the `<exports>` module.
* Only two modules: `<local>` and the facade.
* The local module has all the evaluation side effects and local exports.
* The facade reexports the local module and all reexports of the module.
* `require()` and `import *` will resolve to the facade module
* `import "..."` will resolve to the locals
* `import { x } from "..."` will follow reexports and finally imports from a locals module

Author: sokra

test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts

@@ -2430,7 +2430,7 @@ describe('Cache Components Errors', () => {
                     |                                      ^",
                  "stack": [
                    "{module evaluation} app/use-cache-private-in-unstable-cache/page.tsx (21:38)",
-                   "<FIXME-file-protocol>",
+                   "{module evaluation} .next-internal/server/app/use-cache-private-in-unstable-cache/page/actions.js (server actions loader) (1:1)",
                    "<FIXME-file-protocol>",
                    "<FIXME-next-dist-dir>",
                  ],
@@ -2487,8 +2487,8 @@ describe('Cache Components Errors', () => {
                 expect(output).toMatchInlineSnapshot(`
                  "Error: "use cache: private" must not be used within \`unstable_cache()\`.
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-unstable-cache/page.tsx:21:38)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-unstable-cache/page/actions.js (server actions loader):1:1)
                      at a (<next-dist-dir>)
-                     at b (<next-dist-dir>)
                    19 | }
                    20 |
                  > 21 | const getCachedData = unstable_cache(async () => {
@@ -2506,6 +2506,7 @@ describe('Cache Components Errors', () => {
                 expect(output).toMatchInlineSnapshot(`
                  "Error: "use cache: private" must not be used within \`unstable_cache()\`.
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-unstable-cache/page.tsx:21:38)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-unstable-cache/page/actions.js%20(server%20actions%20loader):1:1)
                      at a (<next-dist-dir>)
                    19 | }
                    20 |
@@ -2574,7 +2575,7 @@ describe('Cache Components Errors', () => {
                     | ^",
                  "stack": [
                    "{module evaluation} app/use-cache-private-in-use-cache/page.tsx (15:1)",
-                   "<FIXME-file-protocol>",
+                   "{module evaluation} .next-internal/server/app/use-cache-private-in-use-cache/page/actions.js (server actions loader) (1:1)",
                    "<FIXME-file-protocol>",
                    "<FIXME-next-dist-dir>",
                  ],
@@ -2632,8 +2633,8 @@ describe('Cache Components Errors', () => {
                 expect(output).toMatchInlineSnapshot(`
                  "Error: "use cache: private" must not be used within "use cache". It can only be nested inside of another "use cache: private".
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:1)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-use-cache/page/actions.js (server actions loader):1:1)
                      at a (<next-dist-dir>)
-                     at b (<next-dist-dir>)
                    13 | }
                    14 |
                  > 15 | async function Private() {
@@ -2643,8 +2644,8 @@ describe('Cache Components Errors', () => {
                    18 |   return <p>Private</p>
                  Error: "use cache: private" must not be used within "use cache". It can only be nested inside of another "use cache: private".
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:1)
-                     at c (<next-dist-dir>)
-                     at d (<next-dist-dir>)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-use-cache/page/actions.js (server actions loader):1:1)
+                     at b (<next-dist-dir>)
                    13 | }
                    14 |
                  > 15 | async function Private() {
@@ -2662,7 +2663,7 @@ describe('Cache Components Errors', () => {
                 expect(output).toMatchInlineSnapshot(`
                  "Error: "use cache: private" must not be used within "use cache". It can only be nested inside of another "use cache: private".
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:1)
-                     at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:16)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-use-cache/page/actions.js%20(server%20actions%20loader):1:1)
                      at a (<next-dist-dir>)
                    13 | }
                    14 |
@@ -2673,7 +2674,7 @@ describe('Cache Components Errors', () => {
                    18 |   return <p>Private</p>
                  Error: "use cache: private" must not be used within "use cache". It can only be nested inside of another "use cache: private".
                      at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:1)
-                     at __TURBOPACK__module__evaluation__ (bundler:///app/use-cache-private-in-use-cache/page.tsx:15:16)
+                     at __TURBOPACK__module__evaluation__ (bundler:///.next-internal/server/app/use-cache-private-in-use-cache/page/actions.js%20(server%20actions%20loader):1:1)
                      at b (<next-dist-dir>)
                    13 | }
                    14 |

turbopack/crates/turbopack-ecmascript/src/chunk/placeable.rs

@@ -230,7 +230,7 @@ pub enum EcmascriptExports {
 #[turbo_tasks::value_impl]
 impl EcmascriptExports {
     #[turbo_tasks::function]
-    pub async fn needs_facade(&self) -> Result<Vc<bool>> {
+    pub async fn split_locals_and_reexports(&self) -> Result<Vc<bool>> {
         Ok(match self {
             EcmascriptExports::EsmExports(exports) => {
                 let exports = exports.await?;

turbopack/crates/turbopack-ecmascript/src/lib.rs

@@ -850,8 +850,8 @@ pub struct EcmascriptModuleContentOptions {
     specified_module_type: SpecifiedModuleType,
     chunking_context: ResolvedVc<Box<dyn ChunkingContext>>,
     references: ResolvedVc<ModuleReferences>,
-    esm_references: ResolvedVc<EsmAssetReferences>,
     part_references: Vec<ResolvedVc<EcmascriptModulePartReference>>,
+    esm_references: ResolvedVc<EsmAssetReferences>,
     code_generation: ResolvedVc<CodeGens>,
     async_module: ResolvedVc<OptionAsyncModule>,
     generate_source_map: bool,
@@ -872,8 +872,8 @@ impl EcmascriptModuleContentOptions {
             module,
             chunking_context,
             references,
-            esm_references,
             part_references,
+            esm_references,
             code_generation,
             async_module,
             exports,
@@ -912,14 +912,14 @@ impl EcmascriptModuleContentOptions {
                 },
             ];
 
-            let esm_code_gens = esm_references
-                .await?
+            let part_code_gens = part_references
                 .iter()
                 .map(|r| r.code_generation(**chunking_context, scope_hoisting_context))
                 .try_join()
                 .await?;
 
-            let part_code_gens = part_references
+            let esm_code_gens = esm_references
+                .await?
                 .iter()
                 .map(|r| r.code_generation(**chunking_context, scope_hoisting_context))
                 .try_join()
@@ -933,9 +933,9 @@ impl EcmascriptModuleContentOptions {
                 .await?;
 
             anyhow::Ok(
-                esm_code_gens
+                part_code_gens
                     .into_iter()
-                    .chain(part_code_gens.into_iter())
+                    .chain(esm_code_gens.into_iter())
                     .chain(additional_code_gens.into_iter().flatten())
                     .chain(code_gens.into_iter())
                     .collect(),

turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs

@@ -141,17 +141,8 @@ pub async fn follow_reexports(
     side_effect_free_packages: Vc<Glob>,
     ignore_side_effect_of_entry: bool,
 ) -> Result<Vc<FollowExportsResult>> {
-    if !ignore_side_effect_of_entry
-        && !*module
-            .is_marked_as_side_effect_free(side_effect_free_packages)
-            .await?
-    {
-        return Ok(FollowExportsResult::cell(FollowExportsResult {
-            module,
-            export_name: Some(export_name),
-            ty: FoundExportType::SideEffects,
-        }));
-    }
+    let mut ignore_side_effects = ignore_side_effect_of_entry;
+
     let mut module = module;
     let mut export_name = export_name;
     loop {
@@ -164,12 +155,26 @@ pub async fn follow_reexports(
             }));
         };
 
+        if !ignore_side_effects
+            && !*module
+                .is_marked_as_side_effect_free(side_effect_free_packages)
+                .await?
+        {
+            // TODO It's unfortunate that we have to use the whole module here.
+            // This is often the Facade module, which includes all reexports.
+            // Often we could use Locals + the followed reexports instead.
+            return Ok(FollowExportsResult::cell(FollowExportsResult {
+                module,
+                export_name: Some(export_name),
+                ty: FoundExportType::SideEffects,
+            }));
+        }
+        ignore_side_effects = false;
+
         // Try to find the export in the local exports
         let exports_ref = exports.await?;
         if let Some(export) = exports_ref.exports.get(&export_name) {
-            match handle_declared_export(module, export_name, export, side_effect_free_packages)
-                .await?
-            {
+            match handle_declared_export(module, export_name, export).await? {
                 ControlFlow::Continue((m, n)) => {
                     module = m.to_resolved().await?;
                     export_name = n;
@@ -222,23 +227,12 @@ async fn handle_declared_export(
     module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
     export_name: RcStr,
     export: &EsmExport,
-    side_effect_free_packages: Vc<Glob>,
 ) -> Result<ControlFlow<FollowExportsResult, (Vc<Box<dyn EcmascriptChunkPlaceable>>, RcStr)>> {
     match export {
         EsmExport::ImportedBinding(reference, name, _) => {
             if let ReferencedAsset::Some(module) =
                 *ReferencedAsset::from_resolve_result(reference.resolve_reference()).await?
             {
-                if !*module
-                    .is_marked_as_side_effect_free(side_effect_free_packages)
-                    .await?
-                {
-                    return Ok(ControlFlow::Break(FollowExportsResult {
-                        module,
-                        export_name: Some(name.clone()),
-                        ty: FoundExportType::SideEffects,
-                    }));
-                }
                 return Ok(ControlFlow::Continue((*module, name.clone())));
             }
         }
@@ -286,6 +280,7 @@ async fn find_export_from_reexports(
     module: ResolvedVc<Box<dyn EcmascriptChunkPlaceable>>,
     export_name: RcStr,
 ) -> Result<Vc<FindExportFromReexportsResult>> {
+    // TODO why do we need a special case for this?
     if let Some(module) =
         Vc::try_resolve_downcast_type::<EcmascriptModulePartAsset>(*module).await?
         && matches!(module.await?.part, ModulePart::Exports)

turbopack/crates/turbopack-ecmascript/src/references/mod.rs

@@ -171,7 +171,6 @@ pub struct AnalyzeEcmascriptModuleResult {
     pub esm_references: ResolvedVc<EsmAssetReferences>,
     pub esm_local_references: ResolvedVc<EsmAssetReferences>,
     pub esm_reexport_references: ResolvedVc<EsmAssetReferences>,
-    pub esm_evaluation_references: ResolvedVc<EsmAssetReferences>,
 
     pub code_generation: ResolvedVc<CodeGens>,
     pub exports: ResolvedVc<EcmascriptExports>,
@@ -217,7 +216,6 @@ pub struct AnalyzeEcmascriptModuleResultBuilder {
     esm_references: FxHashSet<usize>,
     esm_local_references: FxHashSet<usize>,
     esm_reexport_references: FxHashSet<usize>,
-    esm_evaluation_references: FxHashSet<usize>,
 
     esm_references_free_var: FxIndexMap<RcStr, ResolvedVc<EsmAssetReference>>,
     // Ad-hoc created import references that are resolved `import * as x from ...; x.foo` accesses
@@ -239,7 +237,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
             esm_references: Default::default(),
             esm_local_references: Default::default(),
             esm_reexport_references: Default::default(),
-            esm_evaluation_references: Default::default(),
             esm_references_rewritten: Default::default(),
             esm_references_free_var: Default::default(),
             code_gens: Default::default(),
@@ -282,7 +279,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
     pub fn add_esm_evaluation_reference(&mut self, idx: usize) {
         self.esm_references.insert(idx);
         self.esm_local_references.insert(idx);
-        self.esm_evaluation_references.insert(idx);
     }
 
     /// Adds a codegen to the analysis result.
@@ -369,8 +365,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
         });
         let mut esm_reexport_references = track_reexport_references
             .then(|| Vec::with_capacity(self.esm_reexport_references.len()));
-        let mut esm_evaluation_references = track_reexport_references
-            .then(|| Vec::with_capacity(self.esm_evaluation_references.len()));
         for (i, reference) in import_references.iter().enumerate() {
             if self.esm_references.contains(&i) {
                 esm_references.push(*reference);
@@ -392,11 +386,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
                         .flat_map(|m| m.values().copied()),
                 );
             }
-            if let Some(esm_evaluation_references) = &mut esm_evaluation_references
-                && self.esm_evaluation_references.contains(&i)
-            {
-                esm_evaluation_references.push(*reference);
-            }
             if let Some(esm_reexport_references) = &mut esm_reexport_references
                 && self.esm_reexport_references.contains(&i)
             {
@@ -415,9 +404,6 @@ impl AnalyzeEcmascriptModuleResultBuilder {
                 esm_reexport_references: ResolvedVc::cell(
                     esm_reexport_references.unwrap_or_default(),
                 ),
-                esm_evaluation_references: ResolvedVc::cell(
-                    esm_evaluation_references.unwrap_or_default(),
-                ),
                 code_generation: ResolvedVc::cell(self.code_gens),
                 exports: self.exports.resolved_cell(),
                 async_module: self.async_module,