Turbopack: pull in updated vercel/nft tests #88162
Merged
Conversation
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
Collaborator
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
94efff9 to
2003bbd
Compare
...opack-tracing/tests/node-file-trace/test/unit/webpack-wrapper-strs-namespaces-large/input.js
Show resolved
Hide resolved
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
2003bbd to
68180e0
Compare
| @@ -0,0 +1 @@ | |||
| export const test = 'fallback version' | |||
Contributor
There was a problem hiding this comment.
Files use ES module syntax (export const) but package.json declares type: "commonjs", causing SyntaxError at runtime
View Details
📝 Patch Details
diff --git a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/fallback.js b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/fallback.js
index 4e3ede2819..8b3121f685 100644
--- a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/fallback.js
+++ b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/fallback.js
@@ -1 +1 @@
-export const test = 'fallback version'
+module.exports = { test: 'fallback version' }
diff --git a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/module-sync.js b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/module-sync.js
index e7287d8dff..3a14d71e6e 100644
--- a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/module-sync.js
+++ b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs-node20/module-sync.js
@@ -1 +1 @@
-export const test = 'module-sync version'
+module.exports = { test: 'module-sync version' }
diff --git a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/fallback.js b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/fallback.js
index 4e3ede2819..8b3121f685 100644
--- a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/fallback.js
+++ b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/fallback.js
@@ -1 +1 @@
-export const test = 'fallback version'
+module.exports = { test: 'fallback version' }
diff --git a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/module-sync.js b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/module-sync.js
index e7287d8dff..3a14d71e6e 100644
--- a/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/module-sync.js
+++ b/turbopack/crates/turbopack-tracing/tests/node-file-trace/test/unit/module-sync-condition-cjs/module-sync.js
@@ -1 +1 @@
-export const test = 'module-sync version'
+module.exports = { test: 'module-sync version' }
Analysis
The issue was a syntax mismatch in two test directories:
- module-sync-condition-cjs-node20: fallback.js and module-sync.js
- module-sync-condition-cjs: fallback.js and module-sync.js
Both directories have package.json with "type": "commonjs", which tells Node.js to treat .js files as CommonJS modules. However, the files were using ES module syntax (export const test = '...'), which is invalid in CommonJS context.
When input.js calls require('test-pkg-sync-cjs-node20') or require('test-pkg-sync-cjs'), Node.js attempts to parse the fallback.js (as it's in the default export condition) and module-sync.js (as the module-sync condition is used) as CommonJS files. This causes a SyntaxError: "Unexpected token 'export'".
The fix converts the export statements to CommonJS syntax using module.exports:
export const test = 'fallback version'→module.exports = { test: 'fallback version' }export const test = 'module-sync version'→module.exports = { test: 'module-sync version' }
After the fix, both test directories run successfully without syntax errors.
Comment on lines
+117
to
+118
| // #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace" | ||
| // )] #[case::module_create_require_destructure("module-create-require-destructure")] |
Contributor
There was a problem hiding this comment.
Suggested change
| // #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace" | |
| // )] #[case::module_create_require_destructure("module-create-require-destructure")] | |
| // #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace")] | |
| // #[case::module_create_require_destructure("module-create-require-destructure")] |
Malformed comment spanning lines 117-118 with improper attribute macro syntax
View Details
📝 Patch Details
diff --git a/turbopack/crates/turbopack-tracing/tests/unit.rs b/turbopack/crates/turbopack-tracing/tests/unit.rs
index efc3976c5c..7afdcfbd5e 100644
--- a/turbopack/crates/turbopack-tracing/tests/unit.rs
+++ b/turbopack/crates/turbopack-tracing/tests/unit.rs
@@ -114,8 +114,8 @@ static ALLOC: turbo_tasks_malloc::TurboMalloc = turbo_tasks_malloc::TurboMalloc;
// #[case::microtime_node_gyp("microtime-node-gyp")]
// #[case::mixed_esm_cjs("mixed-esm-cjs")]
#[case::module_create_require("module-create-require")]
-// #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace"
-// )] #[case::module_create_require_destructure("module-create-require-destructure")]
+// #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace")]
+// #[case::module_create_require_destructure("module-create-require-destructure")]
// #[case::module_create_require_ignore_other("module-create-require-ignore-other")]
// #[case::module_create_require_named_import("module-create-require-named-import")]
// #[case::module_create_require_named_require("module-create-require-named-require")]
Analysis
The issue was a stylistically malformed comment in the test attribute list. Lines 117-118 attempted to comment out two test case attributes but did so in a confusing and incorrect manner:
Original problematic code (lines 117-118):
// #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace"
// )] #[case::module_create_require_destructure("module-create-require-destructure")]The problem: Line 117's comment is missing the closing parenthesis and bracket )] before the line ends, leaving an incomplete attribute syntax. Line 118 then starts with // )], making it look like orphaned code that's not properly commented.
While the code is technically syntactically valid (because both lines start with //, making everything a comment), this pattern is:
- Confusing and error-prone to maintain
- Doesn't follow the standard single-line-per-case pattern used elsewhere in the file
- Makes the intent unclear to readers
Fix applied:
Changed to properly close each attribute on its own line:
// #[case::module_create_require_destructure_namespace("module-create-require-destructure-namespace")]
// #[case::module_create_require_destructure("module-create-require-destructure")]This makes the code cleaner, more maintainable, and consistent with the surrounding commented-out test cases in the file.
Collaborator
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **430 kB** → **430 kB** ✅ -3 B82 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
bgw
approved these changes
Jan 9, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The unit.rs test fixture isn't exactly doing what the vercel/nft setup is doing. We need to look into which of these are actually broken because of Turbopack bugs
vercel/nft@941be1c...67038d5