Turbopack: write symlinks to access transitive serverExternalPackages

crates/next-core/src/next_server/context.rs

@@ -199,7 +199,6 @@ pub async fn get_server_resolve_options_context(
     external_packages.retain(|item| !transpiled_packages.contains(item));
 
     let server_external_packages_plugin = ExternalCjsModulesResolvePlugin::new(
-        project_path.clone(),
         project_path.root().owned().await?,
         ExternalPredicate::Only(ResolvedVc::cell(external_packages)).cell(),
         *next_config.import_externals().await?,
@@ -225,7 +224,6 @@ pub async fn get_server_resolve_options_context(
         server_external_packages_plugin
     } else {
         ExternalCjsModulesResolvePlugin::new(
-            project_path.clone(),
             project_path.root().owned().await?,
             ExternalPredicate::AllExcept(ResolvedVc::cell(transpiled_packages)).cell(),
             *next_config.import_externals().await?,

crates/next-core/src/next_server/resolve.rs

@@ -41,7 +41,6 @@ pub enum ExternalPredicate {
 /// possible to resolve them at runtime.
 #[turbo_tasks::value]
 pub(crate) struct ExternalCjsModulesResolvePlugin {
-    project_path: FileSystemPath,
     root: FileSystemPath,
     predicate: ResolvedVc<ExternalPredicate>,
     import_externals: bool,
@@ -51,13 +50,11 @@ pub(crate) struct ExternalCjsModulesResolvePlugin {
 impl ExternalCjsModulesResolvePlugin {
     #[turbo_tasks::function]
     pub fn new(
-        project_path: FileSystemPath,
         root: FileSystemPath,
         predicate: ResolvedVc<ExternalPredicate>,
         import_externals: bool,
     ) -> Vc<Self> {
         ExternalCjsModulesResolvePlugin {
-            project_path,
             root,
             predicate,
             import_externals,
@@ -259,104 +256,9 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
             };
             break result_from_original_location;
         };
-        let node_resolved = resolve(
-            self.project_path.clone(),
-            reference_type.clone(),
-            request,
-            node_resolve_options,
-        );
-
-        let Some(result) = *node_resolved.first_source().await? else {
-            // this can't resolve with node.js from the project directory, so bundle it
-            return unable_to_externalize(vec![
-                StyledString::Text(
-                    "The request could not be resolved by Node.js from the project \
-                     directory.\nPackages that should be external need to be installed in the \
-                     project directory, so they can be resolved from the output files.\nTry to \
-                     install it into the project directory by running "
-                        .into(),
-                ),
-                StyledString::Code(format!("npm install {package}").into()),
-                StyledString::Text(rcstr!(" from the project directory.")),
-            ]);
-        };
 
-        if result_from_original_location != result {
-            let package_json_file =
-                find_context_file(result.ident().path().await?.parent(), package_json(), false);
-            let package_json_from_original_location = find_context_file(
-                result_from_original_location.ident().path().await?.parent(),
-                package_json(),
-                false,
-            );
-            let FindContextFileResult::Found(package_json_file, _) = &*package_json_file.await?
-            else {
-                return unable_to_externalize(vec![StyledString::Text(
-                    "The package.json of the package resolved from the project directory can't be \
-                     found."
-                        .into(),
-                )]);
-            };
-            let FindContextFileResult::Found(package_json_from_original_location, _) =
-                &*package_json_from_original_location.await?
-            else {
-                return unable_to_externalize(vec![StyledString::Text(rcstr!(
-                    "The package.json of the package can't be found."
-                ))]);
-            };
-            let FileJsonContent::Content(package_json_file) =
-                &*package_json_file.read_json().await?
-            else {
-                return unable_to_externalize(vec![StyledString::Text(
-                    "The package.json of the package resolved from project directory can't be \
-                     parsed."
-                        .into(),
-                )]);
-            };
-            let FileJsonContent::Content(package_json_from_original_location) =
-                &*package_json_from_original_location.read_json().await?
-            else {
-                return unable_to_externalize(vec![StyledString::Text(rcstr!(
-                    "The package.json of the package can't be parsed."
-                ))]);
-            };
-            let (Some(name), Some(version)) = (
-                package_json_file.get("name").and_then(|v| v.as_str()),
-                package_json_file.get("version").and_then(|v| v.as_str()),
-            ) else {
-                return unable_to_externalize(vec![StyledString::Text(rcstr!(
-                    "The package.json of the package has no name or version."
-                ))]);
-            };
-            let (Some(name2), Some(version2)) = (
-                package_json_from_original_location
-                    .get("name")
-                    .and_then(|v| v.as_str()),
-                package_json_from_original_location
-                    .get("version")
-                    .and_then(|v| v.as_str()),
-            ) else {
-                return unable_to_externalize(vec![StyledString::Text(
-                    "The package.json of the package resolved from project directory has no name \
-                     or version."
-                        .into(),
-                )]);
-            };
-            if (name, version) != (name2, version2) {
-                // this can't resolve with node.js from the original location, so bundle it
-                return unable_to_externalize(vec![StyledString::Text(
-                    format!(
-                        "The package resolves to a different version when requested from the \
-                         project directory ({version}) compared to the package requested from the \
-                         importing module ({version2}).\nMake sure to install the same version of \
-                         the package in both locations."
-                    )
-                    .into(),
-                )]);
-            }
-        }
-        let path = result.ident().path().owned().await?;
-        let file_type = get_file_type(path.clone(), &path).await?;
+        let path = result_from_original_location.ident().path().await?;
+        let file_type = get_file_type((*path).clone(), &path).await?;
 
         let external_type = match (file_type, is_esm) {
             (FileType::UnsupportedExtension, _) => {
@@ -375,18 +277,19 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
             (FileType::CommonJs, false) => ExternalType::CommonJs,
             (FileType::CommonJs, true) => {
                 // It would be more efficient to use an CJS external instead of an ESM external,
-                // but we need to verify if that would be correct (as in resolves to the same file).
+                // but we need to verify if that would be correct (as in resolves to the same
+                // file).
                 let node_resolve_options =
                     node_cjs_resolve_options(lookup_path.root().owned().await?);
                 let node_resolved = resolve(
-                    self.project_path.clone(),
+                    lookup_path.clone(),
                     reference_type.clone(),
                     request,
                     node_resolve_options,
                 );
                 let resolves_equal = if let Some(result) = *node_resolved.first_source().await? {
                     let cjs_path = result.ident().path().owned().await?;
-                    cjs_path == path
+                    cjs_path == *path
                 } else {
                     false
                 };
@@ -407,7 +310,8 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
             // ecmascript with esm is always external
             (FileType::EcmaScriptModule, true) => ExternalType::EcmaScriptModule,
             (FileType::EcmaScriptModule, false) => {
-                // even with require() this resolves to a ESM, which would break node.js, bundle it
+                // even with require() this resolves to a ESM, which would break node.js, bundle
+                // it
                 return unable_to_externalize(vec![StyledString::Text(
                     "The package seems invalid. require() resolves to a EcmaScript module, which \
                      would result in an error in Node.js."
@@ -416,11 +320,14 @@ impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
             }
         };
 
+        let target = result_from_original_location.ident().path().owned().await?;
+
         Ok(ResolveResultOption::some(*ResolveResult::primary(
             ResolveResultItem::External {
                 name: request_str.into(),
                 ty: external_type,
                 traced: ExternalTraced::Traced,
+                target: Some(target),
             },
         )))
     }

crates/next-core/src/next_shared/resolve.rs

@@ -254,6 +254,7 @@ impl AfterResolvePlugin for NextExternalResolvePlugin {
                 name: specifier.clone(),
                 ty: ExternalType::CommonJs,
                 traced: ExternalTraced::Traced,
+                target: None,
             },
         ))))
     }

test/e2e/externals-transitive/app/layout.js

@@ -0,0 +1,8 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <head></head>
+      <body>{children}</body>
+    </html>
+  )
+}

test/e2e/externals-transitive/app/page.js

@@ -0,0 +1,12 @@
+import depA from 'dep-a'
+import depB from 'dep-b'
+
+export default function page() {
+  return (
+    <body>
+      <p>
+        depA: {depA}, depB: {depB.join(', ')}
+      </p>
+    </body>
+  )
+}

test/e2e/externals-transitive/dep-a/index.js

@@ -0,0 +1,3 @@
+import lodash from 'lodash'
+
+export default lodash.VERSION

test/e2e/externals-transitive/dep-a/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "dep-a",
+  "version": "1.0.0",
+  "dependencies": {
+    "lodash": "3.10.1"
+  }
+}

test/e2e/externals-transitive/dep-b/index.js

@@ -0,0 +1,4 @@
+import lodash from 'lodash'
+import pkg from 'lodash/package.json'
+
+export default [lodash.VERSION, pkg.version]

test/e2e/externals-transitive/dep-b/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "dep-b",
+  "version": "1.0.0",
+  "dependencies": {
+    "lodash": "4.17.21"
+  }
+}

test/e2e/externals-transitive/externals-transitive.test.ts

@@ -0,0 +1,63 @@
+import glob from 'glob'
+import path from 'path'
+import fs from 'fs/promises'
+import { nextTestSetup } from 'e2e-utils'
+
+describe('externals-transitive', () => {
+  const { next, isTurbopack, isNextDeploy, isNextStart } = nextTestSetup({
+    files: __dirname,
+    dependencies: require('./package.json').dependencies,
+  })
+
+  it('uses the right version of transitive externals', async () => {
+    const $ = await next.render$('/')
+    const body = $('body > p').text().trim()
+    expect(body).toEqual(`depA: 3.10.1, depB: 4.17.21, 4.17.21`)
+
+    if (!isNextDeploy) {
+      const files = glob.sync('**/*.js', {
+        cwd: path.join(next.testDir, next.distDir, 'server'),
+      })
+      let isLodashBundled = false
+      for (const file of files) {
+        const content = await next.readFile(
+          path.join(next.distDir, 'server', file)
+        )
+        isLodashBundled =
+          isLodashBundled ||
+          // Code
+          content.includes('__lodash_hash_undefined__') ||
+          // Package.json
+          content.includes('Lodash modular utilities.')
+      }
+
+      if (isTurbopack) {
+        // Assert that lodash wasn't bundled. Turbopack creates symlinks to be able to access
+        // transitive dependencies at runtime.
+        expect(isLodashBundled).toBe(false)
+
+        let symlinks = (
+          await fs.readdir(
+            path.join(next.testDir, next.distDir, 'node_modules')
+          )
+        ).filter((file) => file.startsWith('lodash-'))
+
+        expect(symlinks.length).toBeGreaterThanOrEqual(2)
+
+        if (isNextStart) {
+          // Lists the two symlinks in the NFT
+          const trace = (await next.readJSON(
+            '.next/server/app/page.js.nft.json'
+          )) as { files: string[] }
+
+          for (let symlink of symlinks) {
+            expect(trace.files).toContain(`../../node_modules/${symlink}`)
+          }
+        }
+      } else {
+        // Webpack ends up bundling lodash in dep-a
+        expect(isLodashBundled).toBe(true)
+      }
+    }
+  })
+})

test/e2e/externals-transitive/next.config.js

@@ -0,0 +1,8 @@
+module.exports = {
+  serverExternalPackages: ['lodash'],
+  experimental: {
+    turbopackModuleIds: 'named',
+    turbopackMinify: false,
+    turbopackScopeHoisting: false,
+  },
+}

test/e2e/externals-transitive/package.json

@@ -0,0 +1,7 @@
+{
+  "dependencies": {
+    "dep-a": "file:./dep-a",
+    "dep-b": "file:./dep-b",
+    "lodash": "3.10.1"
+  }
+}

test/integration/externals-pages-bundle/test/externals.test.ts

@@ -37,7 +37,7 @@ describe('default', () => {
         }
 
         // we don't know the name of the minified `__turbopack_external_require__`, so we just check the content.
-        expect(allBundles).toContain('"external-package"')
+        expect(allBundles).toMatch(/"external-package(-[0-9a-f]+)?"/)
         expect(allBundles).not.toContain('"external-package content"')
       } else {
         const output = await fs.readFile(

test/production/required-server-files-ssr-404/test/index.test.ts

@@ -43,6 +43,7 @@ describe('Required Server Files', () => {
           if (
             file === 'server' ||
             file === 'required-server-files.json' ||
+            file === 'node_modules' ||
             requiredFilesManifest.files.includes(join('.next', file))
           ) {
             continue