perf: process static assets in parallel #3911

tirojal · 2025-12-30T16:08:31Z

🔗 Linked issue

#3909

❓ Type of change

Process static assets in parallel

📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
🐞 Bug fix (a non-breaking change that fixes an issue)
👌 Enhancement (improving an existing functionality like performance)
✨ New feature (a non-breaking change that adds functionality)
🧹 Chore (updates to the build process or auxiliary tools and libraries)
⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

📝 Checklist

I have linked an issue or discussion.
I have updated the documentation accordingly.

vercel · 2025-12-30T16:08:37Z

Someone is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

coderabbitai · 2025-12-30T16:08:45Z

📝 Walkthrough

Walkthrough

Replaced sequential iteration in src/build/virtual/public-assets.ts with parallel processing via runParallel (concurrency: 25); added per-asset error aggregation and throws a combined error on any failures. runParallel in src/utils/parallel.ts now returns { errors: unknown[] }.

Changes

Cohort / File(s)	Summary
Parallel asset processing `src/build/virtual/public-assets.ts`	Replaced sequential loop with `runParallel` (concurrency: 25); preserved per-asset operations (MIME resolution, path/etag/stat, metadata, optional base64 inline). Collects per-asset errors and throws an aggregated error if any failures occur; added `runParallel` import and updated error-handling flow.
Parallel helper signature `src/utils/parallel.ts`	`runParallel` signature changed to return `Promise<{ errors: unknown[] }>`; adds internal `errors` collection and returns it. Callers must now handle the returned errors object.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'perf: process static assets in parallel' follows conventional commits format with proper 'perf' type prefix and clearly describes the main optimization in the changeset.
Description check	✅ Passed	The description links the related issue #3909 and indicates the enhancement type is for processing static assets in parallel, which directly relates to the changeset modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3e75db and 05bc97e.

📒 Files selected for processing (2)

src/build/virtual/public-assets.ts
src/utils/parallel.ts

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,js,tsx,jsx}: Use pathe for cross-platform path operations instead of Node.js node:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted

Files:

src/utils/parallel.ts
src/build/virtual/public-assets.ts

src/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use unstorage for storage abstraction

Files:

src/utils/parallel.ts
src/build/virtual/public-assets.ts

src/{build,dev,runner,cli}/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use consola for logging in build/dev code; use nitro.logger when available

Files:

src/build/virtual/public-assets.ts

src/build/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Virtual modules must be registered in src/build/virtual.ts

Files:

src/build/virtual/public-assets.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/build/**/*.{ts,js} : Virtual modules must be registered in `src/build/virtual.ts`

📚 Learning: 2025-12-03T19:09:13.905Z

Learnt from: medz
Repo: nitrojs/nitro PR: 3834
File: src/presets/cloudflare/server-entry.ts:63-63
Timestamp: 2025-12-03T19:09:13.905Z
Learning: In the Nitro Cloudflare preset (src/presets/cloudflare/server-entry.ts), native errors from oxc-parser and Node.js readFile operations should be allowed to bubble up naturally without wrapping, as their native error messages are more user-friendly and provide better diagnostic information.

Applied to files:

src/build/virtual/public-assets.ts

🧬 Code graph analysis (1)

src/build/virtual/public-assets.ts (2)

src/utils/parallel.ts (1)

runParallel (1-43)

src/runtime/virtual/server-assets.ts (1)

assets (6-6)

🔇 Additional comments (3)

src/utils/parallel.ts (1)

5-6: Return structure is correct, pending the error collection fix.

The return type and structure are properly set up to propagate errors to callers. Once the errors.push(error) is added in the catch block, this will work as intended.

Also applies to: 41-42

src/build/virtual/public-assets.ts (2)

35-77: Solid parallelization approach.

The implementation is well-structured:

Concurrency of 25 is appropriate for I/O-bound file operations

Using Promise.all for concurrent readFile/stat within each task is a good micro-optimization

Each task writes to a unique key in the shared assets object, avoiding race conditions

Once the error collection bug in src/utils/parallel.ts is fixed (adding errors.push(error) in the catch block), this will properly propagate failures.

79-84: Error aggregation is well-designed.

Using Error.cause to preserve the original errors is good practice for debugging. The combined message with newline separation provides clear visibility into multiple failures.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1634441 and ccebea6.

📒 Files selected for processing (1)

src/build/virtual/public-assets.ts

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,js,tsx,jsx}: Use pathe for cross-platform path operations instead of Node.js node:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted

Files:

src/build/virtual/public-assets.ts

src/{build,dev,runner,cli}/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use consola for logging in build/dev code; use nitro.logger when available

Files:

src/build/virtual/public-assets.ts

src/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use unstorage for storage abstraction

Files:

src/build/virtual/public-assets.ts

src/build/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Virtual modules must be registered in src/build/virtual.ts

Files:

src/build/virtual/public-assets.ts

🧬 Code graph analysis (1)

src/build/virtual/public-assets.ts (2)

src/utils/parallel.ts (1)

runParallel (1-40)

src/runtime/virtual/server-assets.ts (1)

assets (6-6)

🔇 Additional comments (1)

src/build/virtual/public-assets.ts (1)

9-9: Import looks good.

The runParallel import is correctly structured using ESM syntax and a proper relative path.

src/build/virtual/public-assets.ts

pkg-pr-new · 2025-12-30T16:52:46Z

Open in StackBlitz

npm i https://pkg.pr.new/nitrojs/nitro@3911

commit: 05bc97e

tirojal · 2025-12-30T17:29:50Z

Just realized I forgot to unite 2 awaits in to single await Promise.all as I did in v2 PR
I will be able to do that myself only tomorrow morning

tirojal · 2025-12-31T09:50:24Z

Just realized I forgot to unite 2 awaits in to single await Promise.all as I did in v2 PR I will be able to do that myself only tomorrow morning

Done

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/build/virtual/public-assets.ts (1)

35-77: The error handling concern from the previous review remains valid.

The critical issue flagged in the previous review about runParallel silently swallowing errors (rather than failing the build on asset processing failures) still applies to this implementation and should be addressed before merging.

The parallelization approach and concurrency limit of 25 are appropriate for this use case.

🧹 Nitpick comments (1)

src/build/virtual/public-assets.ts (1)

76-76: Optional: Concurrency value could be tunable.

The hard-coded concurrency of 25 is reasonable for most cases, but consider making it configurable (e.g., via an environment variable or nitro option) to allow tuning based on system resources and asset count.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ccebea6 and e3e75db.

📒 Files selected for processing (1)

src/build/virtual/public-assets.ts

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,js,tsx,jsx}: Use pathe for cross-platform path operations instead of Node.js node:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted

Files:

src/build/virtual/public-assets.ts

src/{build,dev,runner,cli}/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use consola for logging in build/dev code; use nitro.logger when available

Files:

src/build/virtual/public-assets.ts

src/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use unstorage for storage abstraction

Files:

src/build/virtual/public-assets.ts

src/build/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Virtual modules must be registered in src/build/virtual.ts

Files:

src/build/virtual/public-assets.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/build/**/*.{ts,js} : Virtual modules must be registered in `src/build/virtual.ts`

📚 Learning: 2025-12-03T19:09:13.905Z

Learnt from: medz
Repo: nitrojs/nitro PR: 3834
File: src/presets/cloudflare/server-entry.ts:63-63
Timestamp: 2025-12-03T19:09:13.905Z
Learning: In the Nitro Cloudflare preset (src/presets/cloudflare/server-entry.ts), native errors from oxc-parser and Node.js readFile operations should be allowed to bubble up naturally without wrapping, as their native error messages are more user-friendly and provide better diagnostic information.

Applied to files:

src/build/virtual/public-assets.ts

🧬 Code graph analysis (1)

src/build/virtual/public-assets.ts (2)

src/utils/parallel.ts (1)

runParallel (1-40)

src/runtime/virtual/server-assets.ts (1)

assets (6-6)

🔇 Additional comments (2)

src/build/virtual/public-assets.ts (2)

9-9: LGTM: Import is correct.

The import properly brings in the runParallel utility using ESM syntax.

44-47: Excellent: Parallel read and stat per asset.

The use of Promise.all to read the file and get stat information concurrently is a good optimization that eliminates sequential I/O waits for each asset.

pi0

Thnx 🚀

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/utils/parallel.ts (1)
21-24: Critical: Errors are not being collected into the array.

The catch block logs the error but never pushes it to the errors array. This means the returned errors array will always be empty, and the caller's error check will never trigger.
🐛 Proposed fix
     .then(() => cb(route))
     .catch((error) => {
+      errors.push(error);
       console.error(error);
     });

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3e75db and 05bc97e.

📒 Files selected for processing (2)

src/build/virtual/public-assets.ts
src/utils/parallel.ts

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,js,tsx,jsx}: Use pathe for cross-platform path operations instead of Node.js node:path
Use ESM and modern JavaScript
Do not add comments explaining what the line does unless prompted

Files:

src/utils/parallel.ts
src/build/virtual/public-assets.ts

src/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use unstorage for storage abstraction

Files:

src/utils/parallel.ts
src/build/virtual/public-assets.ts

src/{build,dev,runner,cli}/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Use consola for logging in build/dev code; use nitro.logger when available

Files:

src/build/virtual/public-assets.ts

src/build/**/*.{ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

Virtual modules must be registered in src/build/virtual.ts

Files:

src/build/virtual/public-assets.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: nitrojs/nitro PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-24T11:45:17.435Z
Learning: Applies to src/build/**/*.{ts,js} : Virtual modules must be registered in `src/build/virtual.ts`

📚 Learning: 2025-12-03T19:09:13.905Z

Learnt from: medz
Repo: nitrojs/nitro PR: 3834
File: src/presets/cloudflare/server-entry.ts:63-63
Timestamp: 2025-12-03T19:09:13.905Z
Learning: In the Nitro Cloudflare preset (src/presets/cloudflare/server-entry.ts), native errors from oxc-parser and Node.js readFile operations should be allowed to bubble up naturally without wrapping, as their native error messages are more user-friendly and provide better diagnostic information.

Applied to files:

src/build/virtual/public-assets.ts

🧬 Code graph analysis (1)

src/build/virtual/public-assets.ts (2)

src/utils/parallel.ts (1)

runParallel (1-43)

src/runtime/virtual/server-assets.ts (1)

assets (6-6)

🔇 Additional comments (3)

src/utils/parallel.ts (1)

5-6: Return structure is correct, pending the error collection fix.

The return type and structure are properly set up to propagate errors to callers. Once the errors.push(error) is added in the catch block, this will work as intended.

Also applies to: 41-42

src/build/virtual/public-assets.ts (2)

35-77: Solid parallelization approach.

The implementation is well-structured:

Concurrency of 25 is appropriate for I/O-bound file operations

Using Promise.all for concurrent readFile/stat within each task is a good micro-optimization

Each task writes to a unique key in the shared assets object, avoiding race conditions

Once the error collection bug in src/utils/parallel.ts is fixed (adding errors.push(error) in the catch block), this will properly propagate failures.

79-84: Error aggregation is well-designed.

Using Error.cause to preserve the original errors is good practice for debugging. The combined message with newline separation provides clear visibility into multiple failures.

fix: process static assets in parallel nitrojs#3909

ccebea6

tirojal requested a review from pi0 as a code owner December 30, 2025 16:08

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

src/build/virtual/public-assets.ts Outdated Show resolved Hide resolved

tirojal mentioned this pull request Dec 30, 2025

process static assets in parallel #3909

Open

pi0 changed the title ~~fix: process static assets in parallel #3909~~ perf: process static assets in parallel Dec 30, 2025

perf: read file and stat in parallel nitrojs#3909

e3e75db

coderabbitai bot reviewed Dec 31, 2025

View reviewed changes

pi0 added 2 commits January 7, 2026 19:13

Merge branch 'main' into fix/3909-process-static-assets-in-parallel

983f1ae

preserve errors

05bc97e

pi0 approved these changes Jan 7, 2026

View reviewed changes

coderabbitai bot reviewed Jan 7, 2026

View reviewed changes

pi0 merged commit 1c2f1c0 into nitrojs:main Jan 7, 2026
10 of 11 checks passed

coderabbitai bot mentioned this pull request Jan 8, 2026

fix: collect errors in utils/parallel #3932 #3933

Merged

8 tasks

Uh oh!

perf: process static assets in parallel #3911

perf: process static assets in parallel #3911

Uh oh!

Conversation

tirojal commented Dec 30, 2025

🔗 Linked issue

❓ Type of change

📚 Description

📝 Checklist

Uh oh!

vercel bot commented Dec 30, 2025

Uh oh!

coderabbitai bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pkg-pr-new bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tirojal commented Dec 30, 2025

Uh oh!

tirojal commented Dec 31, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

pi0 left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

pkg-pr-new bot commented Dec 30, 2025 •

edited

Loading