hapi icon indicating copy to clipboard operation
hapi copied to clipboard

Incorrect Types for decorators

Open feedmypixel opened this issue 1 year ago • 1 comments

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 avatar Aug 23 '24 19:08 feedmypixel

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(…);
});

kanongil avatar Aug 24 '24 09:08 kanongil

@damusix has picked this up and done the work in https://github.com/hapijs/hapi/pull/4538

feedmypixel avatar Jan 25 '25 22:01 feedmypixel