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

refactor(types): deduplicate listen options and export it #4013

Merged
merged 1 commit into from Jul 9, 2022

Conversation

kyranet
Copy link
Contributor

@kyranet kyranet commented Jun 14, 2022

The options interface for FastifyInstance.listen was duplicated, which could potentially lead to type signature and documentation differences between the two instances. To solve this, I have made a new interface to unify the two, so there's only one source of truth for both overloads.

I also added export to it because I found it useful - I maintain a library that uses Fastify and it allows users to configure it by passing an object with all the options, which is one of the supported (and non-deprecated) overloads of the listen function. Currently, we would need a very hacky type to get said options from the overloads.

Click to see the code
import type { FastifyInstance } from 'fastify';

export type FastifyObjectOptions = Omit<Exclude<OverloadedParameters<FastifyInstance['listen']>[0], FastifyUnnecessaryUnionTypes>, 'host'>;

/**
 * The union types extracted by {@link Overloads}
 * that we do not want to include in {@link FastifyObjectOptions}
 */
type FastifyUnnecessaryUnionTypes = string | number | ((...args: any[]) => any) | undefined;

/**
 * Extracts the overloads from methods and transforms them into a union
 * source: {@link https://github.com/microsoft/TypeScript/issues/32164#issuecomment-811608386}
 */
type Overloads<T extends (...args: any[]) => any> = T extends {
	(...args: infer A1): infer R1;
	(...args: infer A2): infer R2;
	(...args: infer A3): infer R3;
	(...args: infer A4): infer R4;
	(...args: infer A5): infer R5;
	(...args: infer A6): infer R6;
}
	? ((...args: A1) => R1) | ((...args: A2) => R2) | ((...args: A3) => R3) | ((...args: A4) => R4) | ((...args: A5) => R5) | ((...args: A6) => R6)
	: T extends {
			(...args: infer A1): infer R1;
			(...args: infer A2): infer R2;
			(...args: infer A3): infer R3;
			(...args: infer A4): infer R4;
			(...args: infer A5): infer R5;
	  }
	? ((...args: A1) => R1) | ((...args: A2) => R2) | ((...args: A3) => R3) | ((...args: A4) => R4) | ((...args: A5) => R5)
	: T extends { (...args: infer A1): infer R1; (...args: infer A2): infer R2; (...args: infer A3): infer R3; (...args: infer A4): infer R4 }
	? ((...args: A1) => R1) | ((...args: A2) => R2) | ((...args: A3) => R3) | ((...args: A4) => R4)
	: T extends { (...args: infer A1): infer R1; (...args: infer A2): infer R2; (...args: infer A3): infer R3 }
	? ((...args: A1) => R1) | ((...args: A2) => R2) | ((...args: A3) => R3)
	: T extends { (...args: infer A1): infer R1; (...args: infer A2): infer R2 }
	? ((...args: A1) => R1) | ((...args: A2) => R2)
	: T extends { (...args: infer A1): infer R1 }
	? (...args: A1) => R1
	: never;

/**
 * Extracts the overloaded parameters from methods
 * source: {@link https://github.com/microsoft/TypeScript/issues/32164#issuecomment-811608386}
 */
type OverloadedParameters<T extends (...args: any[]) => any> = Parameters<Overloads<T>>;

So for ease of development, I think this export would be beneficial, not for us, but quite possibly for a lot of people as well.

Checklist

@github-actions github-actions bot added the typescript TypeScript related label Jun 14, 2022
import { FastifyError } from '@fastify/error'
import { ConstraintStrategy, HTTPVersion } from 'find-my-way'
import * as http from 'http'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was moved due to VSCode's auto-import, should I leave it as-is, or move it back to the top? 😅

Copy link
Member

Choose a reason for hiding this comment

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

I am ok with this change

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
Copy link
Member

mcollina commented Jul 9, 2022

I'm so sorry, I didn't merge straight away, and now this conflicts. Could you rebase?

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

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

LGTM.

@kyranet kyranet force-pushed the types/expose-options-interface branch from ff96205 to 1d5e74c Compare July 9, 2022 13:03
@kyranet
Copy link
Contributor Author

kyranet commented Jul 9, 2022

I'm so sorry, I didn't merge straight away, and now this conflicts. Could you rebase?

No worries, I have just rebased the PR :)

Edit: I made a second force push to add a missing space from the original code.

@kyranet kyranet force-pushed the types/expose-options-interface branch from 1d5e74c to c56d4c8 Compare July 9, 2022 13:08
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

@github-actions
Copy link

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 Jul 10, 2023
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