elringus / imgit

Transform images, video and YouTube links to HTML optimized for web vitals
https://imgit.dev
MIT License
21 stars 1 forks source link

Invalid type declarations for covariant field in AstroIntegration #7

Closed Fryuni closed 10 months ago

Fryuni commented 10 months ago

The type declared for the Astro plugin specifies unknown[] for Vite's plugin config. This is incompatible with the real AstroIntegration type from astro.

astro.config.mts:110:5 - error TS2322: Type 'AstroIntegration' is not assignable to type 'false | AstroIntegration | (false | AstroIntegration | null | undefined)[] | null | undefined'.
  Type 'AstroIntegration' is not assignable to type 'import("/home/lotus/Workspace/blog/node_modules/.pnpm/astro@4.0.9_patch_hash=76zgkjwwmhzrg4rf6mhio72waq_typescript@5.3.3/node_modules/astro/dist/@types/astro").AstroIntegration'.
    The types of 'hooks['astro:config:setup']' are incompatible between these types.
      Type '((options: { injectScript: AstroInjector; updateConfig: (config: { vite: { plugins: unknown[]; }; }) => void; }) => void | Promise<void>) | undefined' is not assignable to type '((options: { config: AstroConfig; command: "build" | "dev" | "preview"; isRestart: boolean; updateConfig: (newConfig: DeepPartial<AstroConfig>) => AstroConfig; ... 8 more ...; logger: AstroIntegrationLogger; }) => void | Promise<...>) | undefined'.
        Type '(options: { injectScript: AstroInjector; updateConfig: (config: { vite: { plugins: unknown[]; }; }) => void; }) => void | Promise<void>' is not assignable to type '(options: { config: AstroConfig; command: "build" | "dev" | "preview"; isRestart: boolean; updateConfig: (newConfig: DeepPartial<AstroConfig>) => AstroConfig; ... 8 more ...; logger: AstroIntegrationLogger; }) => void | Promise<...>'.
          Types of parameters 'options' and 'options' are incompatible.
            Type '{ config: AstroConfig; command: "build" | "dev" | "preview"; isRestart: boolean; updateConfig: (newConfig: DeepPartial<AstroConfig>) => AstroConfig; ... 8 more ...; logger: AstroIntegrationLogger; }' is not assignable to type '{ injectScript: AstroInjector; updateConfig: (config: { vite: { plugins: unknown[]; }; }) => void; }'.
              Types of property 'updateConfig' are incompatible.
                Type '(newConfig: DeepPartial<AstroConfig>) => AstroConfig' is not assignable to type '(config: { vite: { plugins: unknown[]; }; }) => void'.
                  Types of parameters 'newConfig' and 'config' are incompatible.
                    Type '{ vite: { plugins: unknown[]; }; }' is not assignable to type 'DeepPartial<AstroConfig>'.
                      The types of 'vite.plugins' are incompatible between these types.
                        Type 'unknown[]' is not assignable to type 'DeepPartial<PluginOption[] | undefined>'.
                          Type 'unknown' is not assignable to type 'PluginOption'.

110     imgit({}),
Print of the error formatted ![image](https://github.com/elringus/imgit/assets/11063910/32bed9b2-9ae2-4eb6-b27b-331121a4d9a4)

https://github.com/elringus/imgit/blob/e449e6fd12ee89884a23d950fcfd10ef51061e71/src/plugin/astro.ts#L11

That plugins: unknown[] is inside a function parameter, making the type of updateConfig contravariant over it. But the updateConfig field is itself part of a function parameter, meaning that astro:config:setup and by extension AstroIntegration contravariant over updateConfig. Transitively, AstroIntegration is covariant over the plugins field.

This means that assigning this AstroIntegration type over Astro's actual type would require unknown to be assignable to Vite's PluginOption, but of course unknown is not assignable to anything except itself by definition (since it is a top type).

The field should either:

elringus commented 10 months ago

Hi, Thanks for the notice! Somehow it's not reproducing on my end in the Astro sample: https://github.com/elringus/imgit/blob/main/samples/astro/astro.config.mts, though I'd like to solve this anyway. We can't directly import Vite's plugin type, as it's not a dependency of the plugin. And we can't use any, as it violates codefactor linting. So the only solution I guess would be declaring compatible VitePlugin type. Do you happen to know the min. required type structure that will satisfy Astro requirements?

Fryuni commented 10 months ago

I can send a PR with the subtype being used later today 😄

elringus commented 10 months ago

Would be great, thank you!

Fryuni commented 10 months ago

Took me more than I expected to get to this, but it turns out you already have the vite plugin properly typed. Just had to refer to it, so here it is: