vercel/next.js · #99202

Turbopack: Allow webpack loaders to cross filesystem roots when reading dependencies

bgw · merged Sep 26, 202610 files · 134 + / 75 −
test/e2e/app-dir/turbopack-additional-roots/additional-root-dependency-loader.jsadded17 + / 0 −
@@ -0,0 +1,17 @@+const fs = require('node:fs')+const path = require('node:path')++module.exports = async function (source) {+  const linkedPackage = fs.realpathSync(path.join(this.rootContext, 'linked'))+  const siblingRelative = '../../node_modules/sibling/index.js'+  const siblingAbsolute = path.resolve(linkedPackage, siblingRelative)++  this.addDependency(path.resolve(this.rootContext, 'next.config.js'))+  this.addDependency(siblingAbsolute)+  const resolve = this.getResolve()+  const resolved = await resolve(linkedPackage, siblingRelative)+  if (resolved !== siblingAbsolute) {+    throw new Error(`Expected ${siblingAbsolute}, got ${resolved}`)+  }+  return source.replace('unprocessed', 'processed')+}
test/e2e/app-dir/turbopack-additional-roots/app/loader/input.loader.jsadded1 + / 0 −
@@ -0,0 +1 @@+module.exports = 'unprocessed'
test/e2e/app-dir/turbopack-additional-roots/app/loader/page.tsxadded5 + / 0 −
@@ -0,0 +1,5 @@+import value from './input.loader'++export default function Page() {+  return <p id="loader-value">{value}</p>+}
test/e2e/app-dir/turbopack-additional-roots/next.config.js7 + / 0 −
@@ -8,6 +8,13 @@ const nextConfig = {   output: 'standalone',   outputHashSalt: 'adapter-symlink-test',   serverExternalPackages: ['sibling'],+  turbopack: {+    rules: {+      '*.loader.js': {+        loaders: [path.join(__dirname, 'additional-root-dependency-loader.js')],+      },+    },+  },   experimental: {     turbopackAdditionalRoots: {       linkedPackages: {
test/e2e/app-dir/turbopack-additional-roots/turbopack-additional-roots.test.ts5 + / 0 −
@@ -65,6 +65,11 @@ describe('turbopack additional roots', () => {     )   }) +  it('handles absolute paths and an additional root in dependencies from a webpack loader', async () => {+    const browser = await next.browser('/loader')+    expect(await browser.elementByCss('#loader-value').text()).toBe('processed')+  })+   it('reports initialization warnings when startup succeeds', () => {     expect(next.cliOutput).toContain('Invalid Turbopack additional root')     expect(next.cliOutput).not.toContain('overlaps the project root')
turbopack/crates/turbo-tasks-fs/src/disk.rs49 + / 9 −
@@ -454,7 +454,7 @@ struct OptionRcStr(Option<RcStr>); /// Canonicalizes successive prefixes of `target_sys_path`, from the system root toward the full /// path, and passes each canonical prefix together with the untouched suffix to `visit`. ///-/// Helper for [`DiskFileSystem::resolve_link_target_ancestry_slow_path`] and+/// Helper for [`DiskFileSystem::resolve_path_ancestry_slow_path`] and /// [`DiskFileSystem::lookup_in_file_system_map`] async fn visit_canonicalized_ancestry(     target_sys_path: &Path,@@ -631,6 +631,47 @@ impl DiskFileSystem {         })     } +    /// Similar to [`try_from_sys_path`], but allows resolving paths that cross roots and may be in+    /// a different [`DiskFileSystem`] linked via the [`DiskFileSystemMap`].+    ///+    /// Unlike [`try_from_sys_path`], this conversion is not entirely lexical, and we may attempt to+    /// canonicalize some segments of an absolute path.+    ///+    /// Crossing roots should generally only happen at symlink boundaries. If you are not+    /// handling a path that was potentially formed by crossing a symlink boundary, you should+    /// prefer [`try_from_sys_path`].+    ///+    /// [`try_from_sys_path`]: DiskFileSystem::try_from_sys_path+    pub async fn try_from_sys_path_across_roots(+        &self,+        vc_self: ResolvedVc<DiskFileSystem>,+        sys_path: &Path,+        relative_to: &FileSystemPath,+    ) -> Result<Option<FileSystemPath>> {+        debug_assert_eq!(+            relative_to.fs,+            ResolvedVc::upcast(vc_self),+            "`relative_to` must be in the current disk filesystem"+        );++        if let Some(path) = self.try_from_sys_path(vc_self, sys_path, Some(relative_to)) {+            return Ok(Some(path));+        }++        let absolute_path = self+            .to_sys_path_raw(relative_to)+            .join(sys_path)+            .normalize_lexically()?;+        if let Some(path) = self+            .resolve_path_ancestry_slow_path(vc_self, &absolute_path)+            .await?+        {+            return Ok(Some(path));+        }+        self.lookup_in_file_system_map(vc_self, &absolute_path)+            .await+    }+     /// Returns the path as a system [`PathBuf`]. Similar to [`DiskFileSystem::to_sys_path`], but     /// keeps the internal representation as-is.     ///@@ -667,12 +708,11 @@ impl DiskFileSystem {         return sys_path;     } -    /// Used by the slow path of [`DiskFileSystem::read_link`] for absolute link targets. Attempts-    /// to strip the prefix of an absolute symlink target, creating a [`FileSystemPath`] relative to-    /// the [`DiskFileSystem`] root.+    /// Resolves an absolute path whose spelling differs from the [`DiskFileSystem`] root,+    /// creating a [`FileSystemPath`] relative to that root.     ///-    /// Returns [`None`] if the target never reaches the filesystem root or an ancestor can't be-    /// canonicalized. `read_link` treats this as `LinkContent::Invalid`.+    /// Returns [`None`] if the path never reaches the filesystem root or an ancestor can't be+    /// canonicalized.     ///     /// In some cases that absolute path may contain symlinks, different capitalization, or Windows     /// 8.3 short paths. Resolving this requires performing untracked IO outside of the filesystem@@ -681,7 +721,7 @@ impl DiskFileSystem {     ///     /// To avoid performing untracked reads of files outside of the filesystem root, we iteratively     /// canonicalize each prefix of the given `target_sys_path` using a session-dependent task.-    async fn resolve_link_target_ancestry_slow_path(+    async fn resolve_path_ancestry_slow_path(         &self,         vc_self: ResolvedVc<Self>,         target_sys_path: &Path,@@ -701,7 +741,7 @@ impl DiskFileSystem {      /// Looks up a system path in any configured filesystem other than the current filesystem.     ///-    /// Like [`Self::resolve_link_target_ancestry_slow_path`], the fallback handles paths whose+    /// Like [`Self::resolve_path_ancestry_slow_path`], the fallback handles paths whose     /// spelling differs from a configured root.     async fn lookup_in_file_system_map(         &self,@@ -1013,7 +1053,7 @@ impl FileSystem for DiskFileSystem {             // path.             if target_fs_path.is_none() {                 target_fs_path = this-                    .resolve_link_target_ancestry_slow_path(self, &target_sys_path)+                    .resolve_path_ancestry_slow_path(self, &target_sys_path)                     .await?;             }             if target_fs_path.is_none() {
turbopack/crates/turbopack-node/js/src/transforms/postcss.ts5 + / 5 −
@@ -10,7 +10,7 @@ import type { Processor } from 'postcss' import postcss from '@vercel/turbopack/postcss' // @ts-ignore import importedConfig from 'CONFIG'-import { getReadEnvVariables, toPath, type TransformIpc } from './transforms'+import { getReadEnvVariables, type TransformIpc } from './transforms'  let processor: Processor | undefined @@ -99,16 +99,16 @@ export default async function transform(         break       case 'dependency':       case 'missing-dependency':-        filePaths.push(toPath(msg.file))+        filePaths.push(msg.file)         break       case 'build-dependency':-        buildFilePaths.push(toPath(msg.file))+        buildFilePaths.push(msg.file)         break       case 'dir-dependency':-        directories.push([toPath(msg.dir), msg.glob])+        directories.push([msg.dir, msg.glob])         break       case 'context-dependency':-        directories.push([toPath(msg.dir), '**'])+        directories.push([msg.dir, '**'])         break       default:         // TODO: do we need to do anything here?
turbopack/crates/turbopack-node/js/src/transforms/transforms.ts0 + / 18 −
@@ -3,7 +3,6 @@  */  import type { Channel as Ipc } from '../types'-import { relative, isAbsolute, join, sep } from 'path' import { type StructuredError } from '../error' import { type StackFrame } from '../compiled/stacktrace-parser' @@ -49,23 +48,6 @@ export type IpcRequestMessage =  export type TransformIpc = Ipc<IpcInfoMessage, IpcRequestMessage> -const contextDir = process.cwd()-export const toPath = (file: string) => {-  const relPath = relative(contextDir, file)-  if (isAbsolute(relPath)) {-    throw new Error(-      `Cannot depend on path (${file}) outside of root directory (${contextDir})`-    )-  }-  return sep !== '/' ? relPath.replaceAll(sep, '/') : relPath-}-export const fromPath = (path: string) => {-  return join(-    /* turbopackIgnore: true */ contextDir,-    sep !== '/' ? path.replaceAll('/', sep) : path-  )-}- // Patch process.env to track which env vars are read const originalEnv = process.env const readEnvVars = new Set<string>()
turbopack/crates/turbopack-node/js/src/transforms/webpack-loaders.ts9 + / 17 −
@@ -3,18 +3,13 @@ declare const __turbopack_external_require__: { } & ((id: string, thunk: () => any, esm?: boolean) => any)  import type { Channel as Ipc } from '../types'-import { dirname, resolve as pathResolve, relative } from 'path'+import { dirname, resolve as pathResolve } from 'path' import {   StackFrame,   parse as parseStackTrace, } from '../compiled/stacktrace-parser' import { structuredError, type StructuredError } from '../error'-import {-  fromPath,-  getReadEnvVariables,-  toPath,-  type TransformIpc,-} from './transforms'+import { getReadEnvVariables, type TransformIpc } from './transforms' import {   evaluateBundle,   type ImportModuleResult,@@ -211,7 +206,7 @@ const transform = (               ipc                 .sendRequest({                   type: 'trackFileRead',-                  file: relative(contextDir, pathResolve(p)),+                  file: pathResolve(p),                 })                 .then(                   () => {@@ -348,13 +343,13 @@ const transform = (                 .sendRequest({                   type: 'resolve',                   options: rustOptions,-                  lookupPath: toPath(lookupPath),+                  lookupPath,                   request,                 })                 .then((unknownResult) => {                   let result = unknownResult as { path: string }                   if (result && typeof result.path === 'string') {-                    return fromPath(result.path)+                    return result.path                   } else {                     throw Error(                       'Expected { path: string } from resolve request'@@ -409,7 +404,7 @@ const transform = (                const result = (await ipc.sendRequest({                 type: 'importModule',-                lookupPath: toPath(resourceDir),+                lookupPath: resourceDir,                 request: actualRequest,               })) as ImportModuleResult @@ -561,12 +556,9 @@ const transform = (           filePaths: [             ...result.fileDependencies,             ...result.missingDependencies,-          ].map(toPath),-          directories: result.contextDependencies.map((dep) => [-            toPath(dep),-            '**',-          ]),-          buildFilePaths: [...buildDependencies].map(toPath).sort(),+          ],+          directories: result.contextDependencies.map((dep) => [dep, '**']),+          buildFilePaths: [...buildDependencies].sort(),         })         if (err) {           // Resolve loader paths to include in the error message using
turbopack/crates/turbopack-node/src/transforms/webpack.rs36 + / 26 −
@@ -1,4 +1,4 @@-use std::mem::take;+use std::{mem::take, path::Path};  use anyhow::{Context, Result, bail}; use async_trait::async_trait;@@ -17,10 +17,11 @@ use turbo_tasks::{ }; use turbo_tasks_env::ProcessEnv; use turbo_tasks_fs::{-    File, FileContent, FileSystemEntryType, FileSystemPath,+    DiskFileSystem, File, FileContent, FileSystemEntryType, FileSystemPath,     glob::{Glob, GlobOptions},     json::parse_json_with_source_context,     rope::Rope,+    to_sys_path, }; use turbopack_core::{     asset::{Asset, AssetContent},@@ -231,14 +232,23 @@ async fn webpack_loaders_executor(     )) } +async fn loader_path(cwd: &FileSystemPath, path: &str) -> Result<FileSystemPath> {+    let fs = ResolvedVc::try_downcast_type::<DiskFileSystem>(cwd.fs)+        .context("webpack loader working directory must be on a disk filesystem")?;+    fs.await?+        .try_from_sys_path_across_roots(fs, Path::new(path), cwd)+        .await?+        .with_context(|| format!("webpack loader path {path:?} is outside configured roots"))+}++// TODO: This creates a task that's keyed on the list of file paths, which might churn through tasks+// (creating more pressure for task GC). #[turbo_tasks::function] async fn build_files_changed(-    cwd: FileSystemPath,-    paths: Vec<RcStr>,+    paths: Vec<FileSystemPath>,     source: ResolvedVc<Box<dyn Source>>, ) -> Result<Vc<Completion>> {     for path in paths {-        let path = cwd.join(&path)?;         let entry_type = path.get_type().await?;         match &*entry_type {             FileSystemEntryType::File | FileSystemEntryType::NotFound => {@@ -717,22 +727,25 @@ impl EvaluateContext for WebpackLoaderContext {                         .try_join();                     let file_subscriptions = file_paths                         .iter()-                        .map(async |p| self.cwd.join(p)?.read().await)+                        .map(async |p| loader_path(&self.cwd, p).await?.read().await)                         .try_join();                     let build_file_subscriptions = async {-                        build_files_changed(-                            self.cwd.clone(),-                            build_file_paths,-                            *self.context_source_for_issue,-                        )-                        .await?;+                        // Resolve to `FileSystemPath`s before calling the task: system paths+                        // aren't valid task inputs, as they aren't portable across machines.+                        let build_file_paths = build_file_paths+                            .iter()+                            .map(|p| loader_path(&self.cwd, p))+                            .try_join()+                            .await?;+                        build_files_changed(build_file_paths, *self.context_source_for_issue)+                            .await?;                         Ok::<_, anyhow::Error>(())                     };                     let directory_subscriptions = directories                         .iter()                         .map(async |(dir, glob)| {-                            self.cwd-                                .join(dir)?+                            loader_path(&self.cwd, dir)+                                .await?                                 .track_glob(Glob::new(glob.clone(), GlobOptions::default()), false)                                 .await                         })@@ -779,7 +792,7 @@ impl EvaluateContext for WebpackLoaderContext {                 let Some(resolve_options_context) = self.resolve_options_context else {                     bail!("Resolve options are not available in this context");                 };-                let lookup_path = self.cwd.join(&lookup_path)?;+                let lookup_path = loader_path(&self.cwd, &lookup_path).await?;                 let request = Request::parse(Pattern::Constant(request));                 let options = resolve_options(lookup_path.clone(), *resolve_options_context); @@ -793,15 +806,12 @@ impl EvaluateContext for WebpackLoaderContext {                 );                  if let Some(source) = resolved.await?.first_source() {-                    if let Some(path) = self.cwd.get_relative_path_to(&source.ident().await?.path) {-                        Ok(ResponseMessage::Resolve { path })-                    } else {-                        bail!(-                            "Resolving {} in {} ends up on a different filesystem",-                            request.to_string().await?,-                            lookup_path.to_string_ref().await?-                        );-                    }+                    // Always return absolute paths from the resolve function.+                    let path = to_sys_path(source.ident().await?.path.clone())+                        .await?+                        .context("resolved path is not on a disk filesystem")?;+                    let path = path.to_str().context("resolved path is not valid UTF-8")?;+                    Ok(ResponseMessage::Resolve { path: path.into() })                 } else {                     bail!(                         "Unable to resolve {} in {}",@@ -813,14 +823,14 @@ impl EvaluateContext for WebpackLoaderContext {             RequestMessage::TrackFileRead { file } => {                 // Ignore result, we read on the JS side again to prevent some IPC overhead. Still                 // await the read though to cover at least one class of race conditions.-                let _ = &*self.cwd.join(&file)?.read().await?;+                let _ = &*loader_path(&self.cwd, &file).await?.read().await?;                 Ok(ResponseMessage::TrackFileRead {})             }             RequestMessage::ImportModule {                 lookup_path,                 request,             } => {-                let lookup_path = self.cwd.join(&lookup_path)?;+                let lookup_path = loader_path(&self.cwd, &lookup_path).await?;                  let request_vc = Request::parse(Pattern::Constant(request.clone()));                 let origin = PlainResolveOrigin::new(*self.asset_context, lookup_path.join("_")?);