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

chore: add test for fastify-plugin usage #33

Merged
merged 2 commits into from Aug 13, 2022
Merged

chore: add test for fastify-plugin usage #33

merged 2 commits into from Aug 13, 2022

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Aug 12, 2022

Checklist

I tried using FastifyPluginAsyncTypebox and ran into issues with fastify-plugin. This PR is (for now at least) just a failing test.

/cc @sinclairzx81

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Aug 12, 2022

@SimenB Hi! Just had a quick look at this. I think the issue likely resides inside fastify-plugin package where the fp function signature doesn't seem to support varying generic plugin arguments. The current definitions for the fp library are linked below.

https://github.com/fastify/fastify-plugin/blob/master/plugin.d.ts#L3-L17

I'm able to resolve the issue by making the following edits to the above code. (note: these edits where made directly to the plugin.d.ts definition file located inside node_modules on this branch)

Code Modifications
import {
  FastifyPluginAsync,
  FastifyPluginCallback,
  FastifyPluginOptions,
  RawServerDefault,
  FastifyTypeProviderDefault
} from 'fastify'

// ---------------------------------------------------------------------------------
// Utility Types
// ---------------------------------------------------------------------------------

export type ExtractFastifyPluginAsyncParams<T> = T extends FastifyPluginAsync<
  infer Options,
  infer Server,
  infer TypeProvider
> ? {
  Options: Options,
  Server: Server,
  TypeProvider: TypeProvider
} : {
  Options: FastifyPluginOptions,
  Server: RawServerDefault,
  TypeProvider: FastifyTypeProviderDefault,
}

export type ExtractFastifyPluginCallbackParams<T> = T extends FastifyPluginCallback<
  infer Options,
  infer Server,
  infer TypeProvider
> ? {
  Options: Options,
  Server: Server,
  TypeProvider: TypeProvider
} : {
  Options: FastifyPluginOptions,
  Server: RawServerDefault,
  TypeProvider: FastifyTypeProviderDefault,
}

/**
 * This function does three things for you:
 *   1. Add the `skip-override` hidden property
 *   2. Check bare-minimum version of Fastify
 *   3. Pass some custom metadata of the plugin to Fastify
 * @param fn Fastify plugin function
 * @param options Optional plugin options
 */
export default function fp<Plugin, Params extends ExtractFastifyPluginAsyncParams<Plugin>>(fn: Plugin, options?: PluginMetadata): FastifyPluginAsync<Params['Options'], Params['Server'], Params['TypeProvider']>;
export default function fp<Plugin, Params extends ExtractFastifyPluginAsyncParams<Plugin>>(fn: Plugin, options?: string): FastifyPluginAsync<Params['Options'], Params['Server'], Params['TypeProvider']>;
export default function fp<Plugin, Params extends ExtractFastifyPluginCallbackParams<Plugin>>(fn: Plugin, options?: PluginMetadata):  FastifyPluginCallback<Params['Options'], Params['Server'], Params['TypeProvider']>;
export default function fp<Plugin, Params extends ExtractFastifyPluginCallbackParams<Plugin>>(fn: Plugin, options?: string): FastifyPluginCallback<Params['Options'], Params['Server'], Params['TypeProvider']>;

So, the above is a quick draft that should in theory work, but needs review from the Fastify team for overall correctness. Note, I'm not too familiar with the fastify-plugin package, but maybe the above can serve as a starting point to help get things resolved.

@RafaelGSS Any thoughts on updating the fastify-plugin package to support varying generics on these plugins? Maybe there's a better way to express these definitions?

Hope this helps!
S

@sinclairzx81
Copy link
Contributor

@RafaelGSS
Copy link
Member

RafaelGSS commented Aug 12, 2022

@sinclairzx81 / @SimenB could you confirm if the patch fastify/fastify-plugin#192 fixes this issue?

@sinclairzx81
Copy link
Contributor

@RafaelGSS Heya. Can confirm the updates on fastify/fastify-plugin#192 do enable the TS tests on this branch to complete successfully.

package.json Outdated Show resolved Hide resolved
Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
@SimenB SimenB changed the title chore: add failing test for fastify-plugin usage chore: add test for fastify-plugin usage Aug 13, 2022
@SimenB SimenB marked this pull request as ready for review August 13, 2022 07:35
@SimenB
Copy link
Member Author

SimenB commented Aug 13, 2022

Sweet! Good timing with that PR 😀

@SimenB SimenB merged commit 8900f1d into main Aug 13, 2022
@SimenB SimenB deleted the fastify-plugin branch August 13, 2022 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants