Using strict in a project tsconfig.json makes requestContext.get(string) possibly undefined

[silenceisgolden]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

3.25.0

Plugin version

2.2.0

Node.js version

16.x

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

12.0.1 Monterey

Description

With a project using fastify and fastify-request-context, the user could get confused because

declare module 'fastify-request-context' {
    interface RequestContextData {
       a: number;
    }
}

const value = request.requestContext.get('string');

and value is set to number | undefined. In the documentation it states that the value should be set to a type of number, but this is not always the case.

Steps to Reproduce

Create a new folder/nodejs project with these files.

package.json

{
  "name": "fastify-request-context-typescript",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "fastify": "^3.25.0",
    "fastify-request-context": "^2.2.0"
  },
  "devDependencies": {
    "@types/node": "^17.0.0"
  }
}

tsconfig.json

{
    "compilerOptions": {
        "strict": true // the cause of the issue
    }
}

app.ts

import fastify, { FastifyLoggerInstance } from 'fastify';
import { fastifyRequestContextPlugin } from 'fastify-request-context';

import { simpleController } from './a';

declare module 'fastify-request-context' {
    interface RequestContextData {
        logger: FastifyLoggerInstance;
    }
}

const server = fastify();

server.register((app) => {
    app.get('/', {
        handler: simpleController
    });
}, {
    prefix: '/helloworld',
})

server.register(fastifyRequestContextPlugin, {
    defaultStoreValues: {
        logger: server.log,
    },
});

a.ts

import { RouteHandler } from 'fastify';

export const simpleController: RouteHandler = async (req, reply) => {
    const logger = req.requestContext.get('logger');
    
    // logger is possibly undefined, so this is a typescript error
    logger.info('do a thing');

    reply.code(204);
};

The issue is due to this typing I think:

https://github.com/fastify/fastify-request-context/blob/master/index.d.ts#L8

Since the type is RequestContextData[K] | undefined, in strict typing mode I think that this causes logger above in a.ts to be FastifyLoggerInstance | undefined. This is not the case if you add "strictNullChecks": false to the tsconfig.json.

Expected Behavior

Could the .get method just be typed as get<K extends keyof RequestContextData>(key: K): RequestContextData[K]? This seems to work with "strictNullChecks" set to either true (implicitly by "strict": true), or with "strict": true and "strictNullChecks": false.

[climba03003]

The typing is correct, .get is not always guarantee to be exist. It may return undefined.

https://github.com/kibertoad/asynchronous-local-storage/blob/4504e910c5c88f8f52f74fad046ed7fa2bc8cf63/lib/als.ts#L7-L15
https://nodejs.org/api/async_context.html#asynclocalstoragegetstore

[silenceisgolden]

Thanks for the quick response! So that sort of tells me that the docs should at least be updated then? https://github.com/fastify/fastify-request-context/blob/master/README.md where it says

// Type is string
const foo = requestContext.get('foo')
// Type for unspecified keys is any
const bar = requestContext.get('bar')

maybe there could be a comment added that this is only the case when "strictNullChecks": false?

[climba03003]

Would you like to send a Pull Request to improve the documentation?

[silenceisgolden]

For other folks who run into this issue, I am sort of fixing this by redefining the type:

declare module 'fastify-request-context' {
  interface RequestContextData {
    val: string;
    val2?: string;
  }

  interface RequestContext {
    get<K extends keyof RequestContextData>(key: K): RequestContextData[K];
  }
}

I think this is valid because I am setting correctly typed defaults:

import fastify from 'fastify';
import { fastifyRequestContextPlugin } from 'fastify-request-context';

const server = fastify();

server.register(fastifyRequestContextPlugin, {
  defaultStoreValues: {
    val: '',
    // Since `val2` is typed as possibly undefined, we can leave it undefined here.
  },
});

If you decide to set defaultStoreValues with this type fix, you should be fine since TS will error until you get the correctly typed defaults into the defaultStoreValues object for each property. However, if you do not set defaultStoreValues, you can have a situation where the types are telling you that the value will always be defined, but in fact start out as undefined.

[silenceisgolden]

Closing this issue now that the pull request has been merged and it is link to from the README. Please ping me on this thread though if anyone has a better idea of how to work around this issue. Thanks @climba03003 for the discussion.