Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to return value from callback function #87

Open
1 task
helltraitor opened this issue Jul 16, 2023 · 1 comment
Open
1 task

Allow to return value from callback function #87

helltraitor opened this issue Jul 16, 2023 · 1 comment

Comments

@helltraitor
Copy link

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
image

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?
helltraitor added a commit to helltraitor/nuxt-feedme that referenced this issue Jul 22, 2023
At this moment branch totally fixes typing issues with opening an issue

NOTE: opened issue unjs/hookable#87
@kaikaibenkai
Copy link

kaikaibenkai commented Oct 23, 2023

+1 It's really an important feature

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

No branches or pull requests

2 participants