Skip to content

Conversation

@pi0
Copy link
Member

@pi0 pi0 commented Dec 18, 2025

Before runtimes adopting ESM, bundlers and package authors made up a convention to use top level module field to indicate bundler-friendly-ESM entry. This convention then confused and added to the exports object as an export condition.

Neither of the export condition or field is respected by any runtime but only by bundlers. Standard is the exports field and import condition.

This PR adds module to default conditions to be consistent with rolldown and esbuild (rolldown/rolldown#4703) and fixing bundling issues.

@vercel
Copy link

vercel bot commented Dec 18, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
nitro.build Ready Ready Preview, Comment Dec 18, 2025 2:03pm

@coderabbitai
Copy link

coderabbitai bot commented Dec 18, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Removed explicit mainFields configuration from Rolldown and Rollup, added "module" to default export conditions in the resolver, filtered out "module" in Vite externalConditions, and removed hardcoded exportConditions from the standard preset to rely on the centralized defaults.

Changes

Cohort / File(s) Change Summary
Build Tool Configuration
src/build/rolldown/config.ts, src/build/rollup/config.ts
Removed explicit mainFields: ["main"] from resolve/nodeResolve plugin configuration; removed an associated comment. Module resolution now relies on default resolution behavior.
Export Conditions Configuration
src/config/resolvers/export-conditions.ts
Added "module" to the default export conditions set (now ["import","default","module"]) with an explanatory comment about compatibility use.
External Conditions Filtering
src/build/vite/env.ts
Updated externalConditions filtering to exclude "module" in addition to "browser" and "wasm", changing which conditions are considered for externalization.
Preset Configuration
src/presets/standard/preset.ts
Removed hardcoded exportConditions: ["import", "default"] from the standard preset so it uses the centralized resolver defaults.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Focus attention on:
    • Interaction between adding "module" to default export conditions and filtering "module" in Vite externals (possible resolution mismatches)
    • Ensuring presets now inherit the intended defaults after removal of hardcoded exportConditions
    • Verifying removal of mainFields does not regress resolution for existing dependencies

Possibly related PRs

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title follows conventional commits format with 'fix:' prefix and clearly describes the main change of adding the 'module' condition and main fields for bundling.
Description check ✅ Passed The PR description is directly related to the changeset, providing detailed context about the 'module' condition, its historical usage, and the motivation for the changes across multiple files.

📜 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 85a16ef and 96d4f19.

📒 Files selected for processing (2)
  • src/build/vite/env.ts (2 hunks)
  • src/config/resolvers/export-conditions.ts (1 hunks)

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.

Co-authored-by: Daniel Roe <daniel@roe.dev>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f42bc1 and 85a16ef.

📒 Files selected for processing (5)
  • src/build/rolldown/config.ts (0 hunks)
  • src/build/rollup/config.ts (0 hunks)
  • src/build/vite/env.ts (1 hunks)
  • src/config/resolvers/export-conditions.ts (1 hunks)
  • src/presets/standard/preset.ts (0 hunks)
💤 Files with no reviewable changes (3)
  • src/build/rolldown/config.ts
  • src/build/rollup/config.ts
  • src/presets/standard/preset.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: tests-rollup (windows-latest)
  • GitHub Check: tests-rollup (ubuntu-latest)
  • GitHub Check: tests-rolldown (windows-latest)

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 18, 2025

Open in StackBlitz

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

commit: 85a16ef

@pi0 pi0 merged commit 077b79b into main Dec 18, 2025
6 of 9 checks passed
@pi0 pi0 deleted the fix/add-module-condition branch December 18, 2025 14:03
@coderabbitai coderabbitai bot mentioned this pull request Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants