Type definition of callback arguments for `SessionStore.get`

[rktyt]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

1. I would like to have the result value to be optional and allow null or undefined.
2. I would like to have the method removed from the type definition of the result value. Probably only sessionId, encryptedSessionId and user-defined values are needed for the result value.

get(sessionId, callback) {
  getSessionDataFromCustomDatabase(sessionId)
    .then((data) => {
      if (!data) {
        // If the data simply does not exist, I want to do the following
        callback(null, null) // ❌ TS2345: Argument of type 'null' is not assignable to parameter of type 'Session'.
        // or 
        callback(null) // ❌ TS2554: Expected 2 arguments, but got 1.
        // or
        callback() // ❌ TS2554: Expected 2 arguments, but got 0.
        return
      }
      callback(null, data) // ⚠ If you want to follow the type definition, you need to implement touch, save, regenerate, and other methods on the data.
    })
    .catch((error: Error) => {
      // If the process fails, I want to do the following
      callback(error) // ❌ TS2554: Expected 2 arguments, but got 1.
      // or 
      callback(error, null) // ❌ TS2345: Argument of type 'null' is not assignable to parameter of type 'Session'.
    })
}

Currently, I have to do the following, but I feel it is because the original type definition is bad.

get(sessionId, callback) {
  getSessinDataFromCustomDatabase(sessionId)
    .then((data) => {
      if (!data) {
        callback(null, null as unknown as Fastify.Session) // Not at all type-safe.
        return
      }
      callback(null, data as Fastify.Session) // Not at all type-safe.
    })
    .catch((error: Error) => {
      callback(error, null as unknown as Fastify.Session) // Not at all type-safe.
    })
}

Motivation

No response

Example

No response

[Uzlopak]

A PR is welcome. Dont forget to also adding typings tests.

It is planned to publish on friday a new version.

[Eomm]

Released in v10

https://github.com/fastify/session/blob/16ef7eb6c79ecc2342b0611c3aebb2512ca0c24c/types/types.d.ts#L84

[Uzlopak]

I think what he wants ie that CallbackSession needs to have FastifySession and null as second param

[rktyt]

Thanks.

I think what he wants ie that CallbackSession needs to have FastifySession and null as second param

Yes. And omit unnessary methods or properties.

Like this

// Note: I am not sure if the cookie property is necessary.
type UnnessaryRestoreSessionKeys = 'touch' | 'regenerate' | 'destroy' | 'reload' | 'save' | 'set' | 'get' | 'isModified' | 'cookie';
type CallbackSession = (err: Error | null, result: Omit<Fastify.Session, UnnessaryRestoreSessionKeys> | null) => void;

[kgoedecke]

+1 for this.

[Uzlopak]

@kgoedecke

Can you provide a PR?