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

Plugin<T, D> should be a union of 2 other generics, not an intersection AND union #4537

Open
damusix opened this issue Jan 17, 2025 · 2 comments · May be fixed by #4540
Open

Plugin<T, D> should be a union of 2 other generics, not an intersection AND union #4537

damusix opened this issue Jan 17, 2025 · 2 comments · May be fixed by #4540
Labels
bug Bug or defect types TypeScript type definitions

Comments

@damusix
Copy link
Contributor

damusix commented Jan 17, 2025

Runtime

typescript

Runtime version

5

Module version

21

Last module version without issue

n/a

Used with

Other plugins or tools

Any other relevant information

Currently, the typing for Hapi is

export type Plugin<T, D = void> = PluginBase<T, D> & (PluginNameVersion | PluginPackage);

Which suggests to plugin creators to export their package as Plugin<Opts>.

The problem lies in that, when make tooling around Hapi, if I want to signal a dependency on another plugin in Typescript, I can't depend on MyPlugin.name because it is an intersection between PluginNameVersion | PluginPackage.. One doesn't contain name. The following will throw a TS error:

import HapiSwagger from 'hapi-swagger';

export const myThing = (server: Hapi.Server) => {

    // This package exports as `PluginNameVersion`
    server.depedency(HapiSwagger.name); // Property 'name' does not exist on type 'PluginBase<RegisterOptions, void> & PluginPackage'
}

We should have the option to export both:

// Allows proper typing of the specific package use-case
export type NamedPlugin<T, D = void> = PluginBase<T, D> & PluginNameVersion;
export type PackagedPlugin<T, D = void> = PluginBase<T, D> & PluginPackage;

// won't break existing functionality
export type Plugin<T, D = void> = NamedPlugin<T, D> | PackagedPlugin<T, D>;

What are you trying to achieve or the steps to reproduce?

Specificity in the type of plugin exported

What was the result you got?

Property 'name' does not exist on type 'Plugin<RegisterOptions>'.
Property 'name' does not exist on type 'PluginBase<RegisterOptions, void> & PluginPackage'.ts(2339)

What result did you expect?

I expect to be able to use MyPlugin.name with a utility from Hapi itself

@damusix damusix added the bug Bug or defect label Jan 17, 2025
@damusix
Copy link
Contributor Author

damusix commented Jan 17, 2025

Opened as a discussion. I'll make the PR if others agree it makes sense. @Marsup @kanongil thoughts?

@damusix damusix added the types TypeScript type definitions label Jan 17, 2025
@kanongil
Copy link
Contributor

This makes a lot of sense, though it only solves the issue for plugins that are updated to export its subtype. Otherwise you would still need to cast it, as you can right now:

(HapiSwagger as PluginNameVersion).name

Much care should be taken when updating hapi typings that plugins rely on. Otherwise, the plugin could have to choose between supporting the old version, or updated version.

In this case, updated plugins might choose to use the new types for better integration, with the consequence that it can no longer be used with older hapi releases.

FYI, I would prefer if they are declared as interfaces:

export interface NamedPlugin<T, D = void> extends PluginBase<T, D>, PluginNameVersion {}
export interface PackagedPlugin<T, D = void> extends PluginBase<T, D>, PluginPackage {}

Note that plugins can already be nicer about this, as I am in Brok here.

@damusix damusix linked a pull request Jan 20, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug or defect types TypeScript type definitions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants