Skip to content

Conversation

@wadefletch
Copy link
Contributor

@wadefletch wadefletch commented Feb 9, 2026

Linked issue

Resolves #3809

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

Fix hookable type resolution (#3809)

hookable was listed as a devDependency, so consumers installing nitro didn't get it installed. This caused the HookableCore type (used in NitroApp.hooks) to resolve as any, breaking type inference in plugin definitions:

import { definePlugin } from "nitro";

export default definePlugin((nitroApp) => {
  nitroApp.hooks.hook("request", async (event) => {
    // event was typed as `any` instead of `HTTPEvent`
  });
});

hookable is also imported as a runtime value in src/runtime/internal/app.ts, so it belongs in dependencies regardless.

The fix moves hookable from devDependencies to dependencies.

Fix plugin runtime hooks documentation

The "Available hooks" list in the plugins docs had two issues:

  • The response hook signature was documented as (event, { body }) => {} but the actual type is (res, event) => {} (Response first, then HTTPEvent).
  • The close hook was shown in the example code but missing from the available hooks list.

Checklist

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

`hookable` was listed as a devDependency, so consumers couldn't
resolve the `HookableCore` type used in `NitroApp.hooks`, causing
it to be typed as `any` in plugin definitions.

Closes nitrojs#3809

Co-authored-by: Cursor <cursoragent@cursor.com>
@wadefletch wadefletch requested a review from pi0 as a code owner February 9, 2026 23:48
@vercel
Copy link

vercel bot commented Feb 9, 2026

@wadefletch is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Moved hookable from devDependencies to dependencies and removed it from traced packages; updated Nitro runtime docs to add a "close" hook and change the "response" hook signature to (res, event).

Changes

Cohort / File(s) Summary
Dependency Configuration
package.json, build.config.ts
Promoted hookable from devDependencies to dependencies (version ^6.0.1 unchanged) and removed hookable from the tracePkgs list in build config.
Documentation - Nitro hooks
docs/1.docs/50.plugins.md
Added "close", () => {} hook and updated "response" hook signature from ("response", (event, { body }) => {}) to ("response", (res, event) => {}) in Available hooks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses type resolution issues by moving hookable to dependencies, but does not implement the primary objective of exporting defineNitroPlugin from public packages [#3809]. The PR should also export defineNitroPlugin from a public package path to fully resolve issue #3809, or clarify if this is a separate follow-up PR.
Out of Scope Changes check ⚠️ Warning Documentation changes to plugin hooks (close hook addition and response hook signature update) appear out of scope relative to the main objective of moving hookable dependency. Either document why the hook signature changes are necessary for this PR, or move documentation updates to a separate PR focused on plugin documentation.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description is well-detailed and directly related to the changeset, explaining the rationale for moving hookable to dependencies and documenting the plugin hook fixes with code examples and issue references.
Title check ✅ Passed The title follows conventional commits format with 'build:' prefix and clearly describes the main changes: moving hookable to dependencies and fixing plugin hook documentation.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • 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.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Feb 9, 2026

Open in StackBlitz

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

commit: 9e40bc6

The `response` hook signature was documented as `(event, { body })` but the
actual type is `(res: Response, event: HTTPEvent)`. Also adds the `close` hook
which was shown in examples but missing from the available hooks list.

Co-authored-by: Cursor <cursoragent@cursor.com>
@wadefletch wadefletch changed the title fix: move hookable to dependencies for correct type resolution fix: move hookable to dependencies and fix plugin hook docs Feb 10, 2026
@pi0 pi0 changed the title fix: move hookable to dependencies and fix plugin hook docs build: move hookable to dependencies and fix plugin hook docs Feb 10, 2026
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Thnx!

@pi0 pi0 merged commit 2586302 into nitrojs:main Feb 10, 2026
6 of 8 checks passed
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.

v3: defineNitroPlugin is not exported correctly

2 participants