Skip to content

Conversation

@wuiyang
Copy link
Contributor

@wuiyang wuiyang commented Sep 10, 2025

🔗 Linked issue

resolves #33198

📚 Description

Short-circuit comparison with p.dst || normalizeTemplate(p).dst as normalizeTemplate will not overwrite dst if exists. This should slightly improve projects with multiple addTemplates call and avoid additional object creation and garbage collection afterwards.

@wuiyang wuiyang requested a review from danielroe as a code owner September 10, 2025 17:40
@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@coderabbitai
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Updates addTemplate in packages/kit/src/template.ts to refine template de-duplication by destination. The removal predicate now checks p.dst when available; otherwise it derives the destination via normalizeTemplate(p).dst. The comparison is made against the incoming template’s dst. No other functional areas are modified, and no exported/public API signatures are changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4fb61d and b22ea6e.

📒 Files selected for processing (1)
  • packages/kit/src/template.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/kit/src/template.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). (20)
  • GitHub Check: test-fixtures (windows-latest, built, rspack, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-off, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-off, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, built, webpack, default, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-off, json, lts/-1)
  • GitHub Check: test-fixtures (windows-latest, dev, vite, async, manifest-on, json, lts/-1)
  • GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
  • GitHub Check: test-benchmark
  • GitHub Check: release-pkg-pr-new
  • GitHub Check: typecheck (ubuntu-latest, bundler)
  • GitHub Check: test-size
  • GitHub Check: typecheck (windows-latest, bundler)
  • GitHub Check: code

Pre-merge checks (5 passed)

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title “perf(kit): short-circuit when dst info exists” concisely describes the primary change by indicating a performance optimisation in the kit package and specifying the introduction of a short-circuit when destination information is already available.
Linked Issues Check ✅ Passed The implementation fulfils the objectives of issue #33198 by replacing redundant calls to normalizeTemplate with a short-circuit evaluation that uses `p.dst
Out of Scope Changes Check ✅ Passed All changes are confined to the duplicate-detection logic in packages/kit/src/template.ts as specified by the linked issue, and there are no unrelated modifications outside the performance improvement scope.
Description Check ✅ Passed The description clearly relates to the changeset by referencing the linked issue, detailing the implementation of the `p.dst
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b46681 and e4fb61d.

📒 Files selected for processing (1)
  • packages/kit/src/template.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Follow standard TypeScript conventions and best practices

Files:

  • packages/kit/src/template.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: GalacticHypernova
PR: nuxt/nuxt#29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.
🧬 Code graph analysis (1)
packages/kit/src/template.ts (1)
packages/kit/src/utils.ts (1)
  • filterInPlace (12-19)
⏰ 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: code
  • GitHub Check: build

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 10, 2025

Open in StackBlitz

@nuxt/kit

npm i https://pkg.pr.new/@nuxt/kit@33200

nuxt

npm i https://pkg.pr.new/nuxt@33200

@nuxt/rspack-builder

npm i https://pkg.pr.new/@nuxt/rspack-builder@33200

@nuxt/schema

npm i https://pkg.pr.new/@nuxt/schema@33200

@nuxt/vite-builder

npm i https://pkg.pr.new/@nuxt/vite-builder@33200

@nuxt/webpack-builder

npm i https://pkg.pr.new/@nuxt/webpack-builder@33200

commit: b22ea6e

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 10, 2025

CodSpeed Performance Report

Merging #33200 will not alter performance

Comparing wuiyang:main (b22ea6e) with main (4b46681)

Summary

✅ 10 untouched benchmarks

@danielroe danielroe merged commit c82e5a0 into nuxt:main Sep 11, 2025
46 checks passed
@danielroe
Copy link
Member

thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[@nuxt/kit] Potential performance improvement

2 participants