fix(kit): add default values for destructured references in addTypeTemplate hooks #33239

hillaryke · 2025-09-15T14:54:20Z

Fixes issue where nodeReferences, sharedReferences, and references arrays could be undefined when destructured in prepare:types and nitro:prepare:types hooks, causing 'Cannot read properties of undefined (reading "push")' errors during package installation.

This ensures backward compatibility by providing empty arrays as defaults when the hook payload doesn't contain these properties.

🔗 Linked issue

Resolves #33238

📚 Description

This PR fixes a critical issue where running bun install or npm install fails due to undefined reference arrays in the prepare:types and nitro:prepare:types hooks.

Problem:

During package installation, the build process tries to destructure nodeReferences, sharedReferences, and references from hook payloads
When these properties are undefined, attempting to push to them causes runtime errors
This breaks the installation process with 'Cannot read properties of undefined (reading "push")' errors

Solution:

Added default empty arrays (= []) when destructuring these properties in hook payloads
Ensures backward compatibility for existing code that expects these arrays to be available
Allows package installation to complete successfully

Impact:
This fix resolves installation failures and maintains full backward compatibility while preventing runtime errors during the build process.

bolt-new-by-stackblitz · 2025-09-15T14:54:23Z

Run & review this pull request in StackBlitz Codeflow.

pkg-pr-new · 2025-09-15T14:58:11Z

Open in StackBlitz

@nuxt/kit

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

nuxt

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

@nuxt/rspack-builder

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

@nuxt/schema

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

@nuxt/vite-builder

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

@nuxt/webpack-builder

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

commit: a7b8ed4

coderabbitai · 2025-09-15T14:58:59Z

Walkthrough

The change modifies packages/kit/src/template.ts to stop destructuring hook payloads and instead accept a single payload parameter in Nuxt and Nitro hook handlers. Each handler now uses guarded initialization (e.g. (payload.references ||= []).push({ path: template.dst })) for references, nodeReferences and sharedReferences where applicable, preventing errors when those payload properties are undefined. No exported or public API signatures were changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title accurately and concisely describes the primary change in the PR: adding default values for destructured references in addTypeTemplate hooks. It is focused, specific to the developer-visible behaviour, and avoids noise such as file lists or emojis. This makes it clear for a teammate scanning history what the main fix is.
Linked Issues Check	✅ Passed	The modifications in packages/kit/src/template.ts replace destructured hook parameters with a single payload object and use the
Out of Scope Changes Check	✅ Passed	The diff appears limited to guarding hook payload arrays in packages/kit/src/template.ts and does not alter exported or public signatures or introduce unrelated logic in other files. There are no additional feature changes or modifications outside the scope described in the linked issue. Therefore no out-of-scope changes were detected.
Description Check	✅ Passed	The PR description clearly explains the bug, reproduction context, linked issue, and the implemented fix of providing default empty arrays to avoid pushing on undefined in prepare:types and nitro:prepare:types hooks. It states the impact and resolution and maps directly to the changeset. Therefore it is related to the changes and meets the lenient acceptance criteria.
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

📜 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 df8c4f7 and a7b8ed4.

📒 Files selected for processing (1)

packages/kit/src/template.ts (2 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). (2)

GitHub Check: build
GitHub Check: code

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.

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: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b1129e and 6e3b606.

📒 Files selected for processing (1)

packages/kit/src/template.ts (2 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

⏰ 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). (17)

GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, 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, dev, vite, async, manifest-on, js, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
GitHub Check: release-pkg-pr-new
GitHub Check: typecheck (ubuntu-latest, bundler)
GitHub Check: test-benchmark
GitHub Check: typecheck (windows-latest, bundler)
GitHub Check: test-size
GitHub Check: code

packages/kit/src/template.ts

codspeed-hq · 2025-09-15T15:09:31Z

CodSpeed Performance Report

Merging #33239 will not alter performance

_{Comparing hillaryke:fix/addTypeTemplate-undefined-references (a7b8ed4) with main (0b1129e)}

Summary

✅ 10 untouched

danielroe

coderabbit makes a good point - shall we mutate the payload instead of creating a new array?

hillaryke · 2025-09-15T17:30:24Z

coderabbit makes a good point - shall we mutate the payload instead of creating a new array?

yes, the suggestions are good. The current approach with destructuring creates new local variables that don't mutate the original payload, which means the pushed references get lost. Let me apply thm

…mplate hooks Fixes issue where nodeReferences, sharedReferences, and references arrays could be undefined when destructured in prepare:types and nitro:prepare:types hooks, causing 'Cannot read properties of undefined (reading "push")' errors. This ensures backward compatibility by providing empty arrays as defaults when the hook payload doesn't contain these properties.

hillaryke · 2025-09-15T17:36:10Z

coderabbit makes a good point - shall we mutate the payload instead of creating a new array?

I've applied all the suggestions to fix the payload mutation issue and updated the existing commit instead of creating a new one. It now mutates the payload object instead of creating local variables and uses the ||= operator to ensure properties are initialized on the payload

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

packages/kit/src/template.ts (4)
83-85: Prefer nullish assignment over logical OR

Use ??= to only initialise when the field is null/undefined (avoids clobbering any rare falsy-but-valid values).
-      (payload.references ||= []).push({ path: template.dst })
+      (payload.references ??= []).push({ path: template.dst })
88-90: Apply the same nullish assignment here

Consistency and slightly safer semantics.
-      (payload.nodeReferences ||= []).push({ path: template.dst })
+      (payload.nodeReferences ??= []).push({ path: template.dst })
93-95: Apply the same nullish assignment here

Keeps behaviour consistent across all reference arrays.
-      (payload.sharedReferences ||= []).push({ path: template.dst })
+      (payload.sharedReferences ??= []).push({ path: template.dst })
108-110: And for Nitro types as well

Mirror the same pattern in the Nitro hook.
-      (payload.references ||= []).push({ path: template.dst })
+      (payload.references ??= []).push({ path: template.dst })

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e3b606 and df8c4f7.

📒 Files selected for processing (1)

packages/kit/src/template.ts (2 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#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.

⏰ 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: build
GitHub Check: codeql (javascript-typescript)
GitHub Check: code

🔇 Additional comments (1)

packages/kit/src/template.ts (1)

81-96: Good fix: mutations now persist on the shared payload

Switching from param destructuring to mutating the payload solves the lost‑push issue and aligns with the linked objectives. LGTM.

hillaryke requested a review from danielroe as a code owner September 15, 2025 14:54

github-actions bot added 4.x bug labels Sep 15, 2025

hillaryke changed the title ~~fix(kit): add default values for destructured references in addTypeTe…~~ fix(kit): add default values for destructured references in addTypeTemplate hooks Sep 15, 2025

coderabbitai bot reviewed Sep 15, 2025

View reviewed changes

packages/kit/src/template.ts Outdated Show resolved Hide resolved

packages/kit/src/template.ts Outdated Show resolved Hide resolved

packages/kit/src/template.ts Outdated Show resolved Hide resolved

packages/kit/src/template.ts Outdated Show resolved Hide resolved

danielroe reviewed Sep 15, 2025

View reviewed changes

hillaryke force-pushed the fix/addTypeTemplate-undefined-references branch from 6e3b606 to df8c4f7 Compare September 15, 2025 17:32

coderabbitai bot reviewed Sep 15, 2025

View reviewed changes

chore: split into separate lines

a7b8ed4

danielroe approved these changes Sep 15, 2025

View reviewed changes

danielroe merged commit 8e749b2 into nuxt:main Sep 15, 2025
82 of 83 checks passed

github-actions bot mentioned this pull request Sep 15, 2025

v4.1.3 #33235

Merged

hillaryke deleted the fix/addTypeTemplate-undefined-references branch September 16, 2025 07:58

chrisbbreuer mentioned this pull request Oct 6, 2025

chore(deps): update all non-major dependencies stacksjs/ts-maps#399

Closed

1 task

chrisbbreuer mentioned this pull request Oct 14, 2025

chore(deps): update all non-major dependencies stacksjs/ts-maps#408

Closed

1 task

This was referenced Oct 22, 2025

chore(deps): update all non-major dependencies stacksjs/ts-maps#411

Merged

chore(deps): update all non-major dependencies stacksjs/ts-maps#413

Closed

Uh oh!

fix(kit): add default values for destructured references in addTypeTemplate hooks #33239

fix(kit): add default values for destructured references in addTypeTemplate hooks #33239

Uh oh!

Conversation

hillaryke commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Linked issue

📚 Description

Uh oh!

bolt-new-by-stackblitz bot commented Sep 15, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #33239 will not alter performance

Summary

Uh oh!

danielroe left a comment

Choose a reason for hiding this comment

Uh oh!

hillaryke commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hillaryke commented Sep 15, 2025

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

hillaryke commented Sep 15, 2025 •

edited

Loading

pkg-pr-new bot commented Sep 15, 2025 •

edited

Loading

coderabbitai bot commented Sep 15, 2025 •

edited

Loading

codspeed-hq bot commented Sep 15, 2025 •

edited

Loading

hillaryke commented Sep 15, 2025 •

edited

Loading