Skip to content

Conversation

@danielroe
Copy link
Member

πŸ”— Linked issue

❓ Type of change

  • πŸ“– 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

spotted in nuxt/nuxt#33005, this allows custom ignore patterns for public assets dirs.

we would want all files emitted from a build to be included in the client assets, for example,

πŸ“ Checklist

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

@danielroe danielroe requested a review from pi0 as a code owner December 17, 2025 13:37
@vercel
Copy link

vercel bot commented Dec 17, 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 17, 2025 9:23pm

@coderabbitai
Copy link

coderabbitai bot commented Dec 17, 2025

πŸ“ Walkthrough

Walkthrough

Adds an optional per-asset ignore configuration to public asset directories and updates asset scanning/copying to pass that per-asset ignore (or fallback to global) into include-pattern generation.

Changes

Cohort / File(s) Summary
Asset scanning & copying
src/build/assets.ts
scanUnprefixedPublicAssets and copyPublicAssets now pass asset.ignore into getIncludePatterns. getIncludePatterns signature updated to accept ignorePatterns: string[] | false (defaulting to nitro.options.ignore) and uses `(ignorePatterns
Type definition
src/types/config.ts
Added an optional ignore?: false | string[] field to PublicAssetDir with JSDoc: false disables ignore patterns for that directory; an array overrides global nitro.ignore patterns.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check correctness of the updated getIncludePatterns signature and defaulting behavior.
  • Verify call sites in assets.ts pass the new parameter consistently and that per-asset false behaves as intended.
  • Confirm the ignore field is optional in the type and JSDoc accurately reflects behavior.

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 title 'feat: allow custom ignore patterns for public assets' follows conventional commits format with a clear feat type and descriptive summary of the change.
Description check βœ… Passed The description is related to the changeset, explaining the motivation from nuxt/nuxt#33005 and the purpose of allowing custom ignore patterns for public assets.
✨ Finishing touches
  • πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/asset-ignores

πŸ“œ 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 1eb7f9a and 9065df6.

πŸ“’ Files selected for processing (1)
  • src/types/config.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/types/config.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-rolldown (windows-latest)
  • GitHub Check: tests-rollup (windows-latest)
  • GitHub Check: tests-rollup (ubuntu-latest)

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.

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: 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 0d9f782 and 1eb7f9a.

πŸ“’ Files selected for processing (2)
  • src/build/assets.ts (3 hunks)
  • src/types/config.ts (1 hunks)
⏰ 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). (2)
  • GitHub Check: tests-rolldown (ubuntu-latest)
  • GitHub Check: tests-rolldown (windows-latest)
πŸ”‡ Additional comments (2)
src/build/assets.ts (2)

21-21: LGTM! Correctly passes per-asset ignore patterns.

The calls now pass asset.ignore to getIncludePatterns, enabling per-asset ignore configuration. The default parameter in getIncludePatterns ensures backward compatibility when asset.ignore is undefined.

Also applies to: 42-42


67-86: LGTM! Logic correctly handles all ignore pattern scenarios.

The function signature and implementation properly handle:

  • false: Disables ignore patterns via (false || []) β†’ []
  • string[]: Uses the provided patterns
  • undefined: Falls back to nitro.options.ignore via default parameter

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 17, 2025

Open in StackBlitz

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

commit: 9065df6

@pi0 pi0 merged commit a335d7d into main Dec 17, 2025
12 checks passed
@pi0 pi0 deleted the feat/asset-ignores branch December 17, 2025 21:26
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