unjs/hookable

🪝 Awaitable Hooks

hookable's Issues

Typescript: Template class extend Hookable type issue

Environment

hookable: v5.5.3
node: v18.18.0

Reproduction

https://stackblitz.com/edit/hookable-extend-issue?file=src%2Fmain.ts&terminal=dev

Describe the bug

When trying to extend a template class with Hookable, the "callHook" method always returns a type error. It seems it always thinks there's an array ([]) argument passed as second parameter.

The following code:

import { Hookable } from 'hookable';

type MyHooks = {
  test: () => void;
};

class MyTemplateClass<T extends MyHooks = MyHooks> extends Hookable<T> {
  test() {
    this.callHook('test');
  }
}

Gives the following Typescript error for this.callHook('test');:

Argument of type '[]' is not assignable to parameter of type 'Parameters<InferCallback<T, "test">>'

Additional context

No response

Logs

No response

Synchronous Hooks

I would like to call hooks synchronously if possible

We are using WebSocketSubjectConfig to trigger some hooks before serialization.

e.g.

function notAnAsyncSerializer(msg: Data): WebSocketMessage {
  hooks.callHook(Hook.BEFORE_SERIALIZED, msg)
  const serialized = serialize(msg)
  hooks.callHook(Hook.SERIALIZED, serialized)
  return serialized
}

There is no way to convert an async call into a sync call so the only other way would be for the library to support it.

e.g. hooks.callHookSync(Hook.BEFORE_SERIALIZED, msg)

Any ideas?

Additional information

• Would you be willing to help implement this feature?

hook events

Allow inspecting hook calls for (debug) inspection and nonblocking purposes. We can also measure execution time with a flag and provide to event.

API could be like this:

hookable.on(hookNamem, (event) => { /* non blocking */ })
hookable.onAll((event => { })

event = { name: 'hook-name', totalTime: timeInMS }

TypeScript Examples

Would be nice to have examples in the README for using types with Hookable e.g.

import { createHooks } from 'hookable';

export type HookTypes = {
  hello: (value: string) => void;
  ping: (value: string) => string;
};

// Create a hookable instance
const hooks = createHooks<HookTypes>();

// Hook on 'hello'
hooks.hook('hello', (val: string) => console.log(val));

// Call 'hello' hook and pass 'Hello World!'
hooks.callHook('hello', 'Hello World!');

// Hook on 'ping' that returns 'PONG'
hooks.hook('ping', (val: string) => {
  console.log(val);
  return 'PONG';
});

// Call 'ping' hook and pass 'PING'
hooks.callHook('ping', 'PING').then((val) => {
  console.log(val);
});

However I ran into an unexpected issue with the return type... if I set the type to the following it works without errors. But surely I need to specify the return type here right?

ping: (value: string) => void

Argument of type '(val: string) => string' is not assignable to parameter of type 'never'.(2345)

Editor: https://stackblitz.com/edit/typescript-fkfd5n?devtoolsheight=33&file=index.ts

set caller

Context: nuxt/nuxt#19753

We might support something like hookable.setCaller({ sync, async }) to override default callers. (or simple class properties!)

Cannot find module 'hable'

Package main points to lib/hable.js but that file doesnt exists in lib/?

(also package.json has key cintributors instead of contributors)

Allow register hooks with `enforce: before|after`

(currently only solution is to directly unshift hooks)

Example workaround: pi0/nitro-cloudflare-dev@cab495a#diff-edda23e3c99a9f21a2e76aaa83978aef76b28e43fe72e672d9096f4c91351cb0R41

Is there a way to clear all hooks?

like emitter.all.clear() in mitt

Uncaught SyntaxError: Unexpected token {

Environment

Android 8
iOS 13.3

Reproduction

no

Describe the bug

in some low version webview, like andorid 8

try {
} catch {}

is wrong

this should be

try {
} catch (err) {}

Additional context

No response

Logs

Uncaught SyntaxError: Unexpected token {

Fully typed nested/namespaced hooks (for `addHooks`)

Debugger

Context: #37, nuxt/framework#7690

We could expose a utility (more multiple utils?) to enable debugger for a bookable instance.

const hooks = createHooks()

// Start debugging hooks name and timing in console
const debugger = createDebugger(hooks, { /*opts* })

debugger.close() // Stop debugging and remove listeners

Options:

• inspect: Show hook params to the console output (enabled for browsers by default?)
• group: Use console.group/groupEnd wrapper around logs happening during a specific hook
• filter: Filter which hooks to enable debugger for. Can be string prefix or fn.

closed

wrong theme)

Type 'HooksT' does not satisfy the constraint 'Record<string, any>'

Environment

Node: 16.17.1
typescript: 4.9.4
hookable: 5.4.2

Reproduction

Minimal reproduction

Run yarn run build or tsc.

Describe the bug

A PR for this issue will be submitted

When compiling with tsc, everything gets compiled correctly, but we receive this error message:

node_modules/hookable/dist/index.d.ts:65:49 - error TS2344: Type 'HooksT' does not satisfy the constraint 'Record<string, any>'.

65     beforeEach(function_: (event: InferSpyEvent<HooksT>) => void): () => void;
                                                   ~~~~~~

  node_modules/hookable/dist/index.d.ts:46:24
    46 declare class Hookable<HooksT = Record<string, HookCallback>, HookNameT extends HookKeys<HooksT> = HookKeys<HooksT>> {   
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    This type parameter might need an `extends Record<string, any>` constraint.

node_modules/hookable/dist/index.d.ts:66:48 - error TS2344: Type 'HooksT' does not satisfy the constraint 'Record<string, any>'.

66     afterEach(function_: (event: InferSpyEvent<HooksT>) => void): () => void;
                                                  ~~~~~~

  node_modules/hookable/dist/index.d.ts:46:24
    46 declare class Hookable<HooksT = Record<string, HookCallback>, HookNameT extends HookKeys<HooksT> = HookKeys<HooksT>> {
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    This type parameter might need an `extends Record<string, any>` constraint.


Found 2 errors in the same file, starting at: node_modules/hookable/dist/index.d.ts:65

error Command failed with exit code 2.

Additional context

No response

Logs

No response

Allow to return value from callback function

Describe the feature

Introduction

First of all, this is kind a comment for PR #81, and issue (feature request) + discussion topic.

Problem

So, what we have is the two types that resolves function signature for given hook:

1. HookCallback type that is used as type hint and being the part of the next one...
   https://github.com/unjs/hookable/blob/main/src/types.ts#L1

2. InferCallback type that ensures hook callback to be appropriate to HookCallback otherwise sets its type to never.
   https://github.com/unjs/hookable/blob/main/src/hookable.ts#L14-L16

And in PR #81 we have some kind of extension over type determination for caller side:
https://github.com/unjs/hookable/pull/81/commits...

As for me, this patch cannot be applied without changes in HookCallback type. I consider to replace void by any
to make callbacks with returned value appropriated for InferCallback type.

In case if you (authors, maintainers) think that this feature will be suitable for the future vision of hookable package internal structure, I could try to implement it (but for know I will left the check under unchecked).

As for current implementation is allowed to pass values from handlers but values have type any, and exactly that is what DMReal32 aims to fix, but since hook must extend HookCallback type, user must pass (...) => void or an type error will be occurred on hook set.

Example

// nuxt module (types.d.ts or related to it), we want to extend hooks

declare module 'nitropack' {
  interface NitroRuntimeHooks {
    'calculate:void': () => void
    'calculate:number': () => number
  }
}

// playground, hook setting
import type { NitroApp } from 'nitropack'

export default (nitroApp: NitroApp) => {
  nitroApp.hooks.hook('calculate:void', () => {
    return 0
  })

//  InferCallback consider this hook function as never type
  nitroApp.hooks.hook('calculate:number', () => {
//                                        ~~~~~~~
    return 0
  })
}

Image for more clarify representation

Solution(s)

First solution is not the solution for me, since I want to have this feature (already works, but lack of type hints):
Replace Promise<any> by Promise<void> and deny PR #81

Second solution is to deny PR #81 (temporary), and ask for implementation of mentioned notice

Third solution is to approve PR #81 and ask \ assign me \ somebody else for this issue implementation (I thinks this is less desired since only one line requires for changes).

Thanks for attention

Additional information

• Would you be willing to help implement this feature?

Action Required: Fix Renovate Configuration

There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.

Error type: baseBranch not found
Message: The following configured baseBranch could not be found: dev

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): update all non-major dependencies (@types/node, @vitest/coverage-c8, changelogen, eslint, eslint-config-unjs, expect-type, pnpm, typescript, vite, vitest)
• chore(deps): update actions/checkout action to v4
• chore(deps): update actions/setup-node action to v4
• chore(deps): update codecov/codecov-action action to v4
• chore(deps): update devdependency @types/node to v20
• chore(deps): update devdependency eslint to v9
• chore(deps): update devdependency prettier to v3
• chore(deps): update devdependency unbuild to v2
• chore(deps): update devdependency vite to v5
• chore(deps): update devdependency vitest to v1
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

github-actions

.github/workflows/ci.yml

• actions/checkout v3
• actions/setup-node v3
• codecov/codecov-action v3

npm

package.json

• @types/node ^18.16.1
• @vitest/coverage-c8 ^0.30.1
• changelogen ^0.5.3
• eslint ^8.39.0
• eslint-config-unjs ^0.1.0
• expect-type ^0.15.0
• prettier ^2.8.8
• typescript ^5.0.4
• unbuild ^1.2.1
• vite ^4.3.3
• vitest ^0.30.1
• pnpm 8.3.1

---

• Check this box to trigger a request for Renovate to run again on this repository

`callHookSync`

sometimes it may be necessary to call hooks synchronously, or let the calling function handle the result

Debugger handling parallel hook calls with same name

Context: #55

Implementation with has some issues still for tracking parallel hook calls with certain timing cases:

(#id indicates assigned event._id we use to pad and add hidden diff and (!) issue)

#0: |----------------| (logEnd #0)
    ctr=1           ctr=1
#1:     |------------------------| (logEnd #1) (!)
        ctr=2                    ctr=1
#1:                        |----------------------| logEnd(#1) (!)
                           ctr=2                  ctr1=

We also need to add tests for better coverage

Allow differed hook calling

In framework usages, when a hook is registered (.hook()) after being called .callHook with no handlers, callers will lose the opportunity to catch it:

// A: It will be called when reaching B
hooks.hook('myplugin', () => { /* is called */ })

// B: Calls any hooks for `myplugin` are already registed
await hooks.callHook('myplugin', 'foo', 'bar')

// C: Will only by called by subsequent myplugin calls. Missing (B)
hooks.hook('myplugin', () => { /* never called */ })

To solve this issue, we need to kinda record the hook calls and replay them when new hook is registered but this means we need to keep them in memory. Simply solution would be defining an initialization phase to set bookable into a recording state and make it replay immediately (C).

// A: Gets called on line (B)
hooks.hook('myplugin', () => { /* gets called */ })

// Record calls while initializing framework
const callDifferedHooks = hooks.differ()

// B: Calls hook from (A) also internally stores a call to ['myplugin, 'foo', 'bar']
await hooks.callHook('myplugin', 'foo', 'bar')

// C: normally won't be called for call in (B)
hooks.hook('myplugin', () => { /* gets called */ })

// Call hook registered in (C) from recordings
// Stop recording and cleanup memory
await callDifferedHooks()

Alternatives:

• We could instead have hooks.record() / hooks.stop()
  • While I find it simpler, it is also less explicit and encouraging to handle within a scope. differHooks can later accept a prefix as well for multi phased recording of specific hooks rather than all.
• We immediately call hook in line (C) opon registration
  • It makes hooks called faster but also because it is in background, error handling harder for consumer (internally, best we could throw a console error)
• We could have hooks.setup(() => { }) that runs callback for auto record any reply afterwards.
  • This could be based on the implementation above. Might work for simple cases, might make code uglier. Have to try.

Alternative syntax with setup:

// Calling to setup, starts recording calls
const promise = hooks.setup(async () => {
  // A: Gets called on line (B)
  hooks.hook('myplugin', () => { /* gets called */ })

  // B: Calls hook from (A) also internally stores a call to ['myplugin, 'foo', 'bar']
  await hooks.callHook('myplugin', 'foo', 'bar')

  // C: normally won't be called for call in (B)
  hooks.hook('myplugin', () => { /* gets called */ })
})

// Call hook registered in (C) from recordings
// Stop recording and cleanup memory
await promise