Repository: hapijs/hapi (language: javascript)

Issue:
Incorrect Types for decorators
### 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](https://github.com/hapijs/hapi/blob/22377ee7329b553f9ef72ff869940dce05523920/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](https://github.com/hapijs/hapi/blob/master/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](https://github.com/hapijs/hapi/blob/master/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


Discussion / hints:
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:

```ts
server.decorate('server', 'myFunc', function () {

     this.inject(…);
});
```

Fix patch:
diff --git a/lib/server.js b/lib/server.js
index d2a5d5bf7..b5c7e10f3 100755
--- a/lib/server.js
+++ b/lib/server.js
@@ -120,7 +120,7 @@ internals.Server = class {
 
         const existing = this._core.decorations[type].get(property);
         if (options.extend) {
-            Hoek.assert(type !== 'handler', 'Cannot extent handler decoration:', propertyName);
+            Hoek.assert(type !== 'handler', 'Cannot extend handler decoration:', propertyName);
             Hoek.assert(existing, `Cannot extend missing ${type} decoration: ${propertyName}`);
             Hoek.assert(typeof method === 'function', `Extended ${type} decoration method must be a function: ${propertyName}`);
 
@@ -142,7 +142,7 @@ internals.Server = class {
 
             // Request
 
-            Hoek.assert(!this._core.Request.reserved.includes(property), 'Cannot override built-in request interface decoration:', propertyName);
+            Hoek.assert(!this._core.Request.reserved.includes(property), 'Cannot override the built-in request interface decoration:', propertyName);
 
             if (options.apply) {
                 this._core.decorations.requestApply = this._core.decorations.requestApply ?? new Map();
@@ -156,14 +156,14 @@ internals.Server = class {
 
             // Response
 
-            Hoek.assert(!this._core.Response.reserved.includes(property), 'Cannot override built-in response interface decoration:', propertyName);
+            Hoek.assert(!this._core.Response.reserved.includes(property), 'Cannot override the built-in response interface decoration:', propertyName);
             this._core.Response.prototype[property] = method;
         }
         else if (type === 'toolkit') {
 
             // Toolkit
 
-            Hoek.assert(!Toolkit.reserved.includes(property), 'Cannot override built-in toolkit decoration:', propertyName);
+            Hoek.assert(!Toolkit.reserved.includes(property), 'Cannot override the built-in toolkit decoration:', propertyName);
             this._core.toolkit.decorate(property, method);
         }
         else {
diff --git a/lib/types/plugin.d.ts b/lib/types/plugin.d.ts
index 2be5b9cf3..9847f8c8e 100644
--- a/lib/types/plugin.d.ts
+++ b/lib/types/plugin.d.ts
@@ -247,16 +247,9 @@ export interface HandlerDecorationMethod {
     defaults?: RouteOptions | ((method: any) => RouteOptions) | undefined;
 }
 
-/**
- * The general case for decorators added via server.decorate.
- */
-export type DecorationMethod<T> = (this: T, ...args: any[]) => any;
-
 /**
  * An empty interface to allow typings of custom plugin properties.
  */
 
 export interface PluginProperties {
 }
-
-export type DecorateName = string | symbol;
diff --git a/lib/types/server/server.d.ts b/lib/types/server/server.d.ts
index 0970b7c17..4d64710e1 100644
--- a/lib/types/server/server.d.ts
+++ b/lib/types/server/server.d.ts
@@ -12,8 +12,6 @@ import {
     ServerRegisterOptions,
     ServerRegisterPluginObject,
     ServerRegisterPluginObjectArray,
-    DecorateName,
-    DecorationMethod,
     HandlerDecorationMethod,
     PluginProperties
 } from '../plugin';
@@ -53,6 +51,57 @@ import {
 import { ServerOptions } from './options';
 import { ServerState, ServerStateCookieOptions } from './state';
 
+/**
+ * The general case for decorators added via server.decorate.
+ */
+export type DecorationMethod<T> = (this: T, ...args: any[]) => any;
+
+export type DecorateName = string | symbol;
+
+export type DecorationValue = object | any[] | boolean | number | string | symbol | Map<any, any> | Set<any>;
+
+type ReservedRequestKeys = (
+    'server' | 'url' | 'query' | 'path' | 'method' |
+    'mime' | 'setUrl' | 'setMethod' | 'headers' | 'id' |
+    'app' | 'plugins' | 'route' | 'auth' | 'pre' |
+    'preResponses' | 'info' | 'isInjected' | 'orig' |
+    'params' | 'paramsArray' | 'payload' | 'state' |
+    'response' | 'raw' | 'domain' | 'log' | 'logs' |
+    'generateResponse' |
+
+    // Private functions
+    '_allowInternals' | '_closed' | '_core' |
+    '_entity' | '_eventContext' | '_events' | '_expectContinue' |
+    '_isInjected' | '_isPayloadPending' | '_isReplied' |
+    '_route' | '_serverTimeoutId' | '_states' | '_url' |
+    '_urlError' | '_initializeUrl' | '_setUrl' | '_parseUrl' |
+    '_parseQuery'
+
+);
+
+type ReservedToolkitKeys = (
+    'abandon' | 'authenticated' | 'close' | 'context' | 'continue' |
+    'entity' | 'redirect' | 'realm' | 'request' | 'response' |
+    'state' | 'unauthenticated' | 'unstate'
+);
+
+type ReservedServerKeys = (
+    // Public functions
+    'app' | 'auth' | 'cache' | 'decorations' | 'events' | 'info' |
+    'listener' | 'load' | 'methods' | 'mime' | 'plugins' | 'registrations' |
+    'settings' | 'states' | 'type' | 'version' | 'realm' | 'control' | 'decoder' |
+    'bind' | 'control' | 'decoder' | 'decorate' | 'dependency' | 'encoder' |
+    'event' | 'expose' | 'ext' | 'inject' | 'log' | 'lookup' | 'match' | 'method' |
+    'path' | 'register' | 'route' | 'rules' | 'state' | 'table' | 'validator' |
+    'start' | 'initialize' | 'stop' |
+
+    // Private functions
+    '_core' | '_initialize' | '_start' | '_stop' | '_cachePolicy' | '_createCache' |
+    '_clone' | '_ext' | '_addRoute'
+);
+
+type ExceptName<Property, ReservedKeys> = Property extends ReservedKeys ? never : Property;
+
 /**
  *  User-extensible type for application specific state (`server.app`).
  */
@@ -309,14 +358,24 @@ export class Server<A = ServerApplicationState> {
      * @return void;
      * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-serverdecoratetype-property-method-options)
      */
-    decorate(type: 'handler', property: DecorateName, method: HandlerDecorationMethod, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
-    decorate(type: 'request', property: DecorateName, method: (existing: ((...args: any[]) => any)) => (request: Request) => DecorationMethod<Request>, options: {apply: true, extend: true}): void;
-    decorate(type: 'request', property: DecorateName, method: (request: Request) => DecorationMethod<Request>, options: {apply: true, extend?: boolean | undefined}): void;
-    decorate(type: 'request', property: DecorateName, method: DecorationMethod<Request>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
-    decorate(type: 'toolkit', property: DecorateName, method: (existing: ((...args: any[]) => any)) => DecorationMethod<ResponseToolkit>, options: {apply?: boolean | undefined, extend: true}): void;
-    decorate(type: 'toolkit', property: DecorateName, method: DecorationMethod<ResponseToolkit>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
-    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;
+    decorate <P extends DecorateName>(type: 'handler', property: P, method: HandlerDecorationMethod, options?: { apply?: boolean | undefined, extend?: never }): void;
+
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, method: (existing: ((...args: any[]) => any)) => (request: Request) => DecorationMethod<Request>, options: {apply: true, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, method: (request: Request) => DecorationMethod<Request>, options: {apply: true, extend?: boolean | undefined}): void;
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, method: DecorationMethod<Request>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, value: (existing: ((...args: any[]) => any)) => (request: Request) => any, options: {apply: true, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, value: (request: Request) => any, options: {apply: true, extend?: boolean | undefined}): void;
+    decorate <P extends DecorateName>(type: 'request', property: ExceptName<P, ReservedRequestKeys>, value: DecorationValue, options?: never): void;
+
+    decorate <P extends DecorateName>(type: 'toolkit', property: ExceptName<P, ReservedToolkitKeys>, method: (existing: ((...args: any[]) => any)) => DecorationMethod<ResponseToolkit>, options: {apply?: boolean | undefined, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'toolkit', property: ExceptName<P, ReservedToolkitKeys>, method: DecorationMethod<ResponseToolkit>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
+    decorate <P extends DecorateName>(type: 'toolkit', property: ExceptName<P, ReservedToolkitKeys>, value: (existing: ((...args: any[]) => any)) => any, options: {apply?: boolean | undefined, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'toolkit', property: ExceptName<P, ReservedToolkitKeys>, value: DecorationValue, options?: never): void;
+
+    decorate <P extends DecorateName>(type: 'server', property: ExceptName<P, ReservedServerKeys>, method: (existing: ((...args: any[]) => any)) => DecorationMethod<Server>, options: {apply?: boolean | undefined, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'server', property: ExceptName<P, ReservedServerKeys>, method: DecorationMethod<Server>, options?: {apply?: boolean | undefined, extend?: boolean | undefined}): void;
+    decorate <P extends DecorateName>(type: 'server', property: ExceptName<P, ReservedServerKeys>, value: (existing: ((...args: any[]) => any)) => any, options: {apply?: boolean | undefined, extend: true}): void;
+    decorate <P extends DecorateName>(type: 'server', property: ExceptName<P, ReservedServerKeys>, value: DecorationValue, options?: never): void;
 
     /**
      * Used within a plugin to declare a required dependency on other plugins where:


Test patch:
diff --git a/test/server.js b/test/server.js
index 49d1247df..94e95fc0a 100755
--- a/test/server.js
+++ b/test/server.js
@@ -294,7 +294,7 @@ describe('Server', () => {
 
     describe('decorate()', () => {
 
-        it('decorates request', async () => {
+        it('decorates request with function', async () => {
 
             const server = Hapi.server();
 
@@ -316,6 +316,25 @@ describe('Server', () => {
             expect(res.result).to.match(/^.*\:.*\:.*\:.*\:.*$/);
         });
 
+        it('decorates request with object', async () => {
+
+            const server = Hapi.server();
+
+            const customData = { id: '123' };
+
+            server.decorate('request', 'customData', customData);
+
+            server.route({
+                method: 'GET',
+                path: '/',
+                handler: (request) => request.customData
+            });
+
+            const res = await server.inject('/');
+            expect(res.statusCode).to.equal(200);
+            expect(res.result).to.equal({ id: '123' });
+        });
+
         it('decorates request (apply)', async () => {
 
             const server = Hapi.server();
@@ -366,6 +385,26 @@ describe('Server', () => {
             expect(res.result).to.match(/^.*\:.*\:.*\:.*\:.*!$/);
         });
 
+        it('decorates request (extend) with an array', async () => {
+
+            const server = Hapi.server();
+
+            const items = ['one', 'two', 'three'];
+
+            server.decorate('request', 'items', items);
+            server.decorate('request', 'items', (existing) => [...existing, 'four'], { extend: true });
+
+            server.route({
+                method: 'GET',
+                path: '/',
+                handler: (request) => request.items
+            });
+
+            const res = await server.inject('/');
+            expect(res.statusCode).to.equal(200);
+            expect(res.result).to.equal([...items, 'four']);
+        });
+
         it('decorates request (apply + extend)', async () => {
 
             const server = Hapi.server();
@@ -444,6 +483,25 @@ describe('Server', () => {
             expect(res.result.status).to.equal('ok');
         });
 
+        it('decorates toolkit with boolean', async () => {
+
+            const server = Hapi.server();
+
+            const isOk = true;
+
+            server.decorate('toolkit', 'isOk', isOk);
+
+            server.route({
+                method: 'GET',
+                path: '/',
+                handler: (request, h) => h.isOk
+            });
+
+            const res = await server.inject('/');
+            expect(res.statusCode).to.equal(200);
+            expect(res.result).to.equal(true);
+        });
+
         it('add new handler', async () => {
 
             const test = {
@@ -534,7 +592,7 @@ describe('Server', () => {
             expect(() => {
 
                 server.decorate('toolkit', 'redirect', () => { });
-            }).to.throw('Cannot override built-in toolkit decoration: redirect');
+            }).to.throw('Cannot override the built-in toolkit decoration: redirect');
         });
 
         it('decorates server', async () => {
@@ -559,6 +617,30 @@ describe('Server', () => {
             expect(res.result).to.equal('ok');
         });
 
+        it('decorates server with Map', async () => {
+
+            const server = Hapi.server();
+
+            const itemsMap = new Map();
+            itemsMap.set('one', 'One');
+            itemsMap.set('two', 'Two');
+            itemsMap.set('three', 'Three');
+
+            server.decorate('server', 'itemsMap', itemsMap);
+
+            server.route({
+                method: 'GET',
+                path: '/',
+                handler: (request) => request.server.itemsMap
+            });
+
+            const res = await server.inject('/');
+            expect(res.statusCode).to.equal(200);
+            expect(res.result.get('one')).to.equal('One');
+            expect(res.result.get('two')).to.equal('Two');
+            expect(res.result.get('three')).to.equal('Three');
+        });
+
         it('throws on double server decoration', () => {
 
             const server = Hapi.server();
diff --git a/test/types/index.ts b/test/types/index.ts
index 988edffb0..4a438b38d 100644
--- a/test/types/index.ts
+++ b/test/types/index.ts
@@ -10,7 +10,8 @@ import {
     Server,
     ServerRoute,
     server as createServer,
-    ServerRegisterPluginObject
+    ServerRegisterPluginObject,
+    Lifecycle
 } from '../..';
 
 const { expect: check } = lab;
@@ -123,4 +124,94 @@ server.method('test.add', (a: number, b: number) => a + b, {
         segment: 'test-segment',
     },
     generateKey: (a: number, b: number) => `${a}${b}`
-});
\ No newline at end of file
+});
+
+declare module '../..' {
+    interface Request {
+        obj1: {
+            func1(a: number, b: number): number;
+        };
+
+        func2: (a: number, b: number) => number;
+    }
+
+    interface ResponseToolkit {
+        obj2: {
+            func3(a: number, b: number): number;
+        };
+
+        func4: (a: number, b: number) => number;
+    }
+
+    interface Server {
+        obj3: {
+            func5(a: number, b: number): number;
+        };
+
+        func6: (a: number, b: number) => number;
+    }
+}
+
+const theFunc = (a: number, b: number) => a + b;
+const theLifecycleMethod: Lifecycle.Method = () => 'ok';
+
+// Error when decorating existing properties
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('request', 'payload', theFunc));
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('toolkit', 'state', theFunc));
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('server', 'dependency', theFunc));
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('server', 'dependency', theFunc));
+
+server.decorate('handler', 'func1_1', () => theLifecycleMethod);
+server.decorate('handler', 'func1_2', () => theLifecycleMethod, { apply: true });
+
+// Error when extending on handler
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('handler', 'func1_3', () => theLifecycleMethod, { apply: true, extend: true }));
+
+// Error when handler does not return a lifecycle method
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('handler', 'func1_4', theFunc));
+
+// Decorating request with functions
+server.decorate('request', 'func2_1', theFunc);
+server.decorate('request', 'func2_1', () => theFunc, { apply: true, extend: true });
+server.decorate('request', 'func2_2', theFunc, { apply: true });
+server.decorate('request', 'func2_2', theFunc, { extend: true });
+
+// Decorating toolkit with functions
+server.decorate('toolkit', 'func4_1', theFunc);
+server.decorate('toolkit', 'func4_1', theFunc, { apply: true, extend: true });
+server.decorate('toolkit', 'func4_2', theFunc, { apply: true });
+server.decorate('toolkit', 'func4_2', theFunc, { extend: true });
+
+// Decorating server with functions
+server.decorate('server', 'func6_1', theFunc);
+server.decorate('server', 'func6_1', theFunc, { apply: true, extend: true });
+server.decorate('server', 'func6_2', theFunc, { apply: true });
+server.decorate('server', 'func6_2', theFunc, { extend: true });
+
+// Decorating request with objects
+server.decorate('request', 'obj1_1', { func1: theFunc });
+
+// Type error when extending on request with objects
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('request', 'obj1_1', { func1: theFunc }, { apply: true, extend: true }));
+
+
+// Decorating toolkit with objects
+server.decorate('toolkit', 'obj2_1', { func3: theFunc });
+
+// Error when extending on toolkit with objects
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('toolkit', 'obj2_1', { func3: theFunc }, { apply: true, extend: true }));
+
+// Decorating server with objects
+server.decorate('server', 'obj3_1', { func5: theFunc });
+
+// Error when extending on server with objects
+// @ts-expect-error Lab does not support overload errors
+check.error(() => server.decorate('server', 'obj3_1', { func5: theFunc }, { apply: true, extend: true }));