feat(pack): Support auto public path in UtooPack#2953
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements an 'auto' public path feature, which enables the runtime to automatically determine the base URL for assets based on the source of the executing script. The changes span the core configuration logic in Rust, the TypeScript build plugins, and the ECMAScript runtime. Feedback from the review points out that the string matching for the 'runtime' and 'auto' configuration keywords should be improved to handle trailing slashes, ensuring that user-provided values like 'auto/' are correctly identified as special modes rather than literal paths.
| match public_path.as_str() { | ||
| "runtime" => return Ok(Vc::cell("__RUNTIME_PUBLIC_PATH__".into())), | ||
| "auto" => return Ok(Vc::cell("__AUTO_PUBLIC_PATH__".into())), | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
The current implementation of computed_public_path handles the special values "runtime" and "auto" before enforcing a trailing slash. This is correct as these are internal markers. However, if a user provides "auto/" or "runtime/" in their configuration, they will fall through to the default case and be returned as literal paths with a trailing slash (e.g., "auto/"), which is likely not the intended behavior. Consider normalizing the input by trimming trailing slashes before matching against these keywords to be more resilient to user input.
| function normalizeHtmlPublicPath(globalPublicPath: string | undefined): string { | ||
| return globalPublicPath === "runtime" || globalPublicPath === "auto" | ||
| ? "" | ||
| : (globalPublicPath ?? ""); | ||
| } |
There was a problem hiding this comment.
The normalizeHtmlPublicPath function correctly returns an empty string for "runtime" and "auto" modes, which is appropriate for static HTML injection where assets are typically co-located. However, similar to the Rust implementation, it only matches exact strings. If a user provides "auto/" in their configuration, it will return "auto/", leading to broken URLs like auto//main.js during asset injection. Consider using a more robust check or normalizing the input string.
References
- Ensure that configuration keywords are handled consistently and resiliently across different parts of the codebase. (link)
Summary
output.publicPath: "auto"public_path/autosnapshot fixture and refresh affected runtime snapshotsnext.jssubmodule pointer to include the Turbopack runtime support from feat(turbopack): Support auto public path in Turbopack runtime next.js#149Why
The existing default publicPath behavior resolves to
/, whilepublicPath: "runtime"readsglobalThis.publicPath. This adds a webpack-compatible"auto"mode for deployments where chunks and assets should resolve relative to the current runtime script URL.Validation
cargo run -p pack-schemaUPDATE=1 cargo test -p pack-tests public_path -- --nocapturecargo test -p pack-tests public_path -- --nocapturenpm run build --workspace @utoo/pack-sharednpm run build:js --workspace @utoo/packcargo fmtcargo clippy --all-targets -- -D warnings --no-depsgit diff --checkgit -C next.js diff --check