Skip to content

feat(deps): upgrade hookable to v6#33905

Merged
danielroe merged 4 commits intomainfrom
chore/hookable
Mar 6, 2026
Merged

feat(deps): upgrade hookable to v6#33905
danielroe merged 4 commits intomainfrom
chore/hookable

Conversation

@danielroe
Copy link
Copy Markdown
Member

🔗 Linked issue

📚 Description

splitting this out from #33005 to address any regressions separately

@bolt-new-by-stackblitz
Copy link
Copy Markdown

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

@danielroe danielroe marked this pull request as draft December 17, 2025 22:50
@danielroe danielroe changed the title chore(deps): upgrade hookable to v6 feat(deps): upgrade hookable to v6 Dec 17, 2025
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Dec 17, 2025

Open in StackBlitz

@nuxt/kit

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

@nuxt/nitro-server

npm i https://pkg.pr.new/@nuxt/nitro-server@33905

nuxt

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

@nuxt/rspack-builder

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

@nuxt/schema

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

@nuxt/vite-builder

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

@nuxt/webpack-builder

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

commit: 8863b52

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 17, 2025

Merging this PR will not alter performance

✅ 20 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing chore/hookable (c83dbd9) with main (8e76613)

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@danielroe danielroe mentioned this pull request Dec 18, 2025
18 tasks
@socket-security
Copy link
Copy Markdown

socket-security bot commented Dec 18, 2025

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

@danielroe danielroe marked this pull request as ready for review March 5, 2026 23:33
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds an experimental asyncCallHook option to schema and config template generation and wires it into runtime. Hook invocation sites in components now use optional chaining when attaching .catch to avoid invoking .catch on undefined. Core runtime can wrap callHook/callHookParallel results in Promise.resolve when asyncCallHook is enabled, and close is changed to an async function that awaits the close hook. Documentation and tests updated accordingly.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.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 accurately describes the main change: upgrading the hookable dependency to v6, which is reflected throughout the changeset in hook handling modifications.
Description check ✅ Passed The description is related to the changeset, explaining that this upgrade was split out from another PR to address regressions separately, which aligns with the dependency upgrade changes.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/hookable

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.

@danielroe danielroe enabled auto-merge March 6, 2026 13:50
Copy link
Copy Markdown
Contributor

@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: 2

Caution

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

⚠️ Outside diff range comments (1)
packages/nuxt/src/app/nuxt.ts (1)

381-389: ⚠️ Potential issue | 🟠 Major

Server-side hooks always return Promises, preventing compat-v5 sync-hook mode.

Because contextCaller is async, the server branch returns a Promise whenever hooks are registered, even when experimental.asyncCallHook is false. This creates an asymmetry with the client branch, which respects the flag and preserves synchronous execution when false. Consequently, the compat-v5 synchronous hook path cannot be exercised on the server without architectural changes to how context is preserved.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/nuxt/src/app/nuxt.ts` around lines 381 - 389, The server branch uses
an async contextCaller which forces Promise returns and breaks compat-v5
sync-hook mode; replace it with two variants and choose based on
experimental.asyncCallHook: keep the existing async function
(contextCallerAsync) that awaits each hook for async mode, and add a synchronous
contextCallerSync that calls each hook inside nuxtApp.runWithContext without
using await (e.g., for (const hook of hooks) hook(...args)), then set
nuxtApp.hooks.callHook to use callHookWith(contextCallerSync, ...) when
nuxtApp.options.experimental.asyncCallHook is false and to use the async variant
otherwise, referencing contextCaller (or named variants),
nuxtApp.hooks.callHook, and nuxtApp.hooks.callHookWith so the server preserves
synchronous execution when requested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/nuxt/src/app/nuxt.ts`:
- Around line 390-392: The current replacement of nuxtApp.hooks.callHook wraps
the call synchronously using Promise.resolve(_callHook(name, ...args)) which
still allows synchronous throws to escape; update the override so it defers
execution into the microtask queue (e.g., use Promise.resolve().then(() =>
_callHook(name, ...args))) thereby ensuring any synchronous exceptions from
_callHook(name, ...args) are converted into rejected promises; modify the
assignment that references _callHook and asyncCallHook accordingly.

In `@packages/nuxt/src/core/nuxt.ts`:
- Around line 62-68: When options.experimental.asyncCallHook is true the current
use of Promise.resolve(runWithNuxtContext(...)) can create a Promise with a void
union and also lets synchronous errors escape, so change both assignments to
create the Promise first and defer invocation using Promise.resolve().then(() =>
runWithNuxtContext(...callHook...)). For callHookParallel ensure the resolved
value is narrowed to Promise<any[]> by using the nullish-coalescing fallback
(callHookParallel(...) ?? []) inside the .then to handle no-hook cases. Update
hooks.callHook and hooks.callHookParallel to use Promise.resolve().then(...)
with runWithNuxtContext and apply ?? [] for callHookParallel.

---

Outside diff comments:
In `@packages/nuxt/src/app/nuxt.ts`:
- Around line 381-389: The server branch uses an async contextCaller which
forces Promise returns and breaks compat-v5 sync-hook mode; replace it with two
variants and choose based on experimental.asyncCallHook: keep the existing async
function (contextCallerAsync) that awaits each hook for async mode, and add a
synchronous contextCallerSync that calls each hook inside nuxtApp.runWithContext
without using await (e.g., for (const hook of hooks) hook(...args)), then set
nuxtApp.hooks.callHook to use callHookWith(contextCallerSync, ...) when
nuxtApp.options.experimental.asyncCallHook is false and to use the async variant
otherwise, referencing contextCaller (or named variants),
nuxtApp.hooks.callHook, and nuxtApp.hooks.callHookWith so the server preserves
synchronous execution when requested.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f5321778-e0a8-4871-909c-e207e2233573

📥 Commits

Reviewing files that changed from the base of the PR and between 95bde83 and 2af438f.

⛔ Files ignored due to path filters (4)
  • packages/kit/package.json is excluded by !**/package.json
  • packages/nuxt/package.json is excluded by !**/package.json
  • packages/schema/package.json is excluded by !**/package.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !pnpm-lock.yaml
📒 Files selected for processing (8)
  • docs/1.getting-started/18.upgrade.md
  • packages/nuxt/src/app/components/nuxt-link.ts
  • packages/nuxt/src/app/components/nuxt-root.vue
  • packages/nuxt/src/app/nuxt.ts
  • packages/nuxt/src/core/nuxt.ts
  • packages/nuxt/src/core/templates.ts
  • packages/schema/src/config/experimental.ts
  • packages/schema/src/types/schema.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/nuxt/src/app/components/nuxt-root.vue

Copy link
Copy Markdown
Contributor

@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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/nuxt/src/app/nuxt.ts`:
- Line 389: The hook wrapper passes the entire args array as a single argument
to callHookWith, causing nested arrays; update nuxtApp.hooks.callHook so it
forwards arguments using the spread operator to callHookWith (i.e.,
callHookWith(contextCaller, name, ...args)) so that callHookWith and
contextCaller receive individual variadic args rather than a single array;
modify the line assigning nuxtApp.hooks.callHook to use ...args and keep the
existing contextCaller and name parameters unchanged.

In `@packages/nuxt/src/core/nuxt.ts`:
- Line 80: The hook registration currently invokes nuxt.close() without awaiting
its Promise; update the hook callback in builder code that calls
nuxt.hook('prepare:types', ...) so it either returns the Promise or is declared
async and awaits nuxt.close() to ensure the 'close' hook completes; reference
the close method on the nuxt object and the nuxt.hook('prepare:types', ...)
registration when applying the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1ae560fc-811b-4602-b1c8-6310558aef84

📥 Commits

Reviewing files that changed from the base of the PR and between db0cb41 and c83dbd9.

📒 Files selected for processing (3)
  • packages/nuxt/src/app/nuxt.ts
  • packages/nuxt/src/core/nuxt.ts
  • test/mocks/nuxt-config.ts

hook: hooks.hook,
ready: () => runWithNuxtContext(nuxt, () => initNuxt(nuxt)),
close: () => hooks.callHook('close', nuxt),
close: async () => { await hooks.callHook('close', nuxt) },
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# First, let's find the nuxt.close() method definition
echo "=== Finding nuxt.close() method definition ==="
rg -n "close:\s*async" packages/nuxt/src/core/nuxt.ts -A 2 -B 2

echo -e "\n=== Checking the full method context around line 80 ==="
sed -n '75,85p' packages/nuxt/src/core/nuxt.ts

Repository: nuxt/nuxt

Length of output: 671


🏁 Script executed:

#!/bin/bash

# Find all nuxt.close() calls in the codebase
echo "=== All nuxt.close() calls ==="
rg -n "nuxt\.close\(\)" packages/nuxt/src --type ts -B 2 -A 2

Repository: nuxt/nuxt

Length of output: 403


🏁 Script executed:

#!/bin/bash

# Check the specific example in builder.ts line 62
echo "=== Checking builder.ts around line 62 ==="
sed -n '58,68p' packages/nuxt/src/core/builder.ts

Repository: nuxt/nuxt

Length of output: 373


🏁 Script executed:

#!/bin/bash

# Search for all close() method calls (not just nuxt.close())
echo "=== All .close() calls in packages/nuxt/src ==="
rg -n "\.close\(\)" packages/nuxt/src --type ts -B 1 -A 1

echo -e "\n=== Check if there are other places calling nuxt.close ==="
rg -n "\.close\(" packages/nuxt/src --type ts | head -20

Repository: nuxt/nuxt

Length of output: 2534


🏁 Script executed:

#!/bin/bash

# Check the broader context of the builder.ts issue - where does prepare:types get called
echo "=== Check where prepare:types hook is called ==="
rg -n "prepare:types" packages/nuxt/src --type ts -B 2 -A 2

Repository: nuxt/nuxt

Length of output: 3248


Ensure nuxt.close() call site properly awaits the async method.

The close method is async and awaits the 'close' hook. The call at packages/nuxt/src/core/builder.ts (line 62) registers a hook callback without awaiting the result:

builder.ts line 62
nuxt.hook('prepare:types', () => nuxt.close())

Since nuxt.close() returns a Promise that is not awaited, the cleanup scheduled by the hook will not complete before the callback returns, potentially leaving cleanup incomplete on process exit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/nuxt/src/core/nuxt.ts` at line 80, The hook registration currently
invokes nuxt.close() without awaiting its Promise; update the hook callback in
builder code that calls nuxt.hook('prepare:types', ...) so it either returns the
Promise or is declared async and awaits nuxt.close() to ensure the 'close' hook
completes; reference the close method on the nuxt object and the
nuxt.hook('prepare:types', ...) registration when applying the change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that's not the case - nuxt.hook('prepare:types', () => nuxt.close()) means that when prepare:types is called, it awaits the close hook

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@danielroe, you're right — I apologize for the incorrect analysis. Since hookable's callHook awaits any Promise returned by a hook callback, returning nuxt.close() from the callback is sufficient to ensure the 'close' hook completes before the prepare:types hook finishes. No explicit async/await inside the callback is needed.


✏️ Learnings added
Learnt from: danielroe
Repo: nuxt/nuxt PR: 33905
File: packages/nuxt/src/core/nuxt.ts:80-80
Timestamp: 2026-03-06T14:21:08.891Z
Learning: In `nuxt/nuxt`, when registering a hook callback via `nuxt.hook(...)`, hookable's `callHook` automatically awaits any Promise returned by the callback. Therefore, patterns like `nuxt.hook('prepare:types', () => nuxt.close())` are correct and sufficient — the returned Promise from `nuxt.close()` will be awaited by hookable before proceeding, ensuring cleanup completes.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: danielroe
Repo: nuxt/nuxt PR: 33905
File: packages/nuxt/src/app/nuxt.ts:389-389
Timestamp: 2026-03-06T14:19:09.012Z
Learning: In hookable v6 (unjs/hookable), `callHookWith` changed its signature from variadic spread args to a single pre-formed array parameter: `callHookWith(caller, name, args)` instead of v5's `callHookWith(caller, name, ...args)`. The caller function also gains a third `name` argument in v6: `(hooks, args, name) => any`. In `packages/nuxt/src/app/nuxt.ts`, passing `args` directly (not spread) to `callHookWith` is intentional and correct for hookable v6 compatibility.

@danielroe danielroe added this pull request to the merge queue Mar 6, 2026
Merged via the queue into main with commit 157e42b Mar 6, 2026
61 of 62 checks passed
@danielroe danielroe deleted the chore/hookable branch March 6, 2026 14:50
This was referenced Mar 6, 2026
danielroe added a commit that referenced this pull request Mar 8, 2026
danielroe added a commit that referenced this pull request Mar 9, 2026
@github-actions github-actions bot mentioned this pull request Mar 10, 2026
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.

2 participants