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

fix generic constraint type #3221

Merged
merged 1 commit into from Aug 1, 2021
Merged

Conversation

matthyk
Copy link
Contributor

@matthyk matthyk commented Jul 31, 2021

Since find-my-way version 4.1 it is possible to specify a generic type for the value of a constraint. The default type of this generic parameter is string. This makes it impossible to register constraints that have a value type other than string. The following example produces the error Type 'ConstraintStrategy<HTTPVersion.V1, number>' is not assignable to type 'ConstraintStrategy<HTTPVersion.V1, string>'. Type 'number' is not assignable to type 'string'..

import fastify from 'fastify'
import { ConstraintStrategy, HTTPVersion, Req, Handler} from 'find-my-way'

const constraintStrategy: ConstraintStrategy<HTTPVersion.V1, number> = {
  deriveConstraint<Context>(req: Req<HTTPVersion.V1>, ctx: Context | undefined): number {
    return 0
  },
  mustMatchWhenDerived: false,
  name: "myStrategy",
  storage() {
    return {
      get(value: number): Handler<HTTPVersion.V1> | null {
        return null
      },
      set(value: number, handler: Handler<HTTPVersion.V1>): void {},
      del(value: number): void {},
      empty(): void {}
    }
  },
  validate(value: unknown): void {
  }
}

const app = fastify({
  constraints: {
    constraintStrategy
  }
})

Setting the generic type to unknown solves this issue.

Checklist

@github-actions github-actions bot added the typescript TypeScript related label Jul 31, 2021
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 1aff021 into fastify:main Aug 1, 2021
This was referenced Aug 2, 2021
@fstoerkle
Copy link

This change is not compatible with find-my-way@4.0.0, I encounter this error when running tsc:

Error: node_modules/fastify/fastify.d.ts(125,21): error TS2314: Generic type 'ConstraintStrategy<V>' requires 1 type argument(s).

Upgrading find-my-way to 4.3.3 fixed this for me.

Shouldn‘t the minimum version of find-my-way be updated to 4.1.0 in fastify's package.json?

@matthyk
Copy link
Contributor Author

matthyk commented Aug 3, 2021

I think you're right. We should upgrade to find-my-way version 4.1.0.

@github-actions
Copy link

github-actions bot commented Aug 4, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
typescript TypeScript related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants