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

Incorrect Types for decorators #4524

Open
feedmypixel opened this issue Aug 23, 2024 · 1 comment · May be fixed by #4525
Open

Incorrect Types for decorators #4524

feedmypixel opened this issue Aug 23, 2024 · 1 comment · May be fixed by #4525
Labels
types TypeScript type definitions

Comments

@feedmypixel
Copy link

feedmypixel commented Aug 23, 2024

Runtime

Node.js

Runtime version

v20.11.1

Module version

@hapi/hapi v21.3.10

What are you trying to achieve or the steps to reproduce?

Trying to add an object via a decorator to server. Whilst using JSDOC/TypeScript and we are getting an error we have to disable.

The below example is concerned with server decorate, but this is also appears to be an issue for all Interfaces bar the Handler

What we can do

When using decorators you can provide any value as the method argument and Hapis internals will add it to the interface being decorated.

fEx:

server.decorate('server', 'isSomethingOn', true)
server.decorate('server', 'anObject', { name: 'ben' })

The example in Hapi code for a server decorator can be seen in lib/server.js#L182

// Server

if (typeof property === "string") {
    Hoek.assert(!Object.getOwnPropertyNames(internals.Server.prototype).includes(property), "Cannot override the built-in server interface method:", propertyName);
} else {
    Hoek.assert(!Object.getOwnPropertySymbols(internals.Server.prototype).includes(property), "Cannot override the built-in server interface method:", propertyName);
}

this._core.instances.forEach((server) => {

    server[property] = method;
});

What the Types say you can do

However the Types only allow for functions:

lib/types/server/server.d.ts#L312-L319

decorate(type: 'server', property: DecorateName, method: (existing: ((...args: any[]) => any)) => DecorationMethod<Server>, options: {apply?: boolean | undefined, extend: true}): void;
decorate(type: 'server', property: DecorateName, method: DecorationMethod<Server>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;

And the docs hint that it is intended that the method argument is used with other values:

method - the extension function or other value.
lib/types/server/server.d.ts#L304

Are we using this correctly?

  • Are we using decorators correctly?
  • Is adding an object via a decorators method argument to Server correct?
  • Is this better put on Server.app? Or Request.app?

Can we raise a PR?

Is an update to the Types around decorators to allow non functions to be passed to method a PR you would accept?

What was the result you got?

We have to bypass decorators that have a non function value provided to the method argument with // @ts-expect-error

What result did you expect?

According to the code, to be able to add any value via a decorators method argument

@feedmypixel feedmypixel added the bug Bug or defect label Aug 23, 2024
@kanongil kanongil added types TypeScript type definitions and removed bug Bug or defect labels Aug 24, 2024
@kanongil
Copy link
Contributor

I agree. The typings are incorrect, confirmed by the API docs, and the code and tests.

Except for type: 'handler', the accepted type is any, as the value is just assigned without any checks.

A PR to fix this would be appreciated.

When fixing this, take care to preserve the existing behaviour of adding the correct this value to inline methods, so something like this continues to be properly typed:

server.decorate('server', 'myFunc', function () {

     this.inject();
});

feedmypixel added a commit to feedmypixel/hapi that referenced this issue Aug 26, 2024
- The code currently allows any value to be added as a decorator, but the
types do not. This work allows any value to be added to a decorator by
updating the types to reflect what the code doeis
- Add relevant tests
- Update inline docs
- Create and export DecorationValue type
- Fixes hapijs#4524
@feedmypixel feedmypixel linked a pull request Aug 26, 2024 that will close this issue
feedmypixel added a commit to feedmypixel/hapi that referenced this issue Aug 26, 2024
- The code currently allows any value to be added as a decorator, but the
types do not. This work allows any value to be added to a decorator by
updating the types to reflect what the code does
- Add relevant tests
- Update inline docs
- Create and export DecorationValue type
- Fixes hapijs#4524
feedmypixel added a commit to feedmypixel/hapi that referenced this issue Aug 26, 2024
- The code currently allows any value to be added as a decorator, but the
types do not. This work allows any value to be added to a decorator by
updating the types to reflect what the code does
- Add relevant tests
- Update inline docs
- Create and export DecorationValue type
- Fixes hapijs#4524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types TypeScript type definitions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants