HiDeoo / starlight-image-zoom

Starlight plugin adding zoom capabilities to your documentation images
https://starlight-image-zoom.vercel.app
MIT License
47 stars 4 forks source link

Suppressing "you already have a `MarkdownContent` component override" warning #18

Closed KianNH closed 2 months ago

KianNH commented 2 months ago

Describe the bug

Whilst manually importing and rendering the ImageZoom component reintroduces the functionality if you have a MarkdownContent override, it doesn't suppress the warning.

https://github.com/HiDeoo/starlight-image-zoom/blob/b46bc1305877b0e98a1129116c87a78f12c61a42/packages/starlight-image-zoom/index.ts#L39-L46

I thought it might be that I should import it and then remove the plugin from astro.config.mjs but that results in an error, since there is no virtual module for the configuration anymore.

To Reproduce

  1. Install the starlight-image-zoom plugin.
  2. Create a custom MarkdownContent override.
  3. Receive the "manually render starlight-image-zoom/components/ImageZoom.astro" warning.
  4. Add and import ImageZoom.astro to your MarkdownContent.astro override.
  5. Receive the same warning.

Expected behavior

Ability to suppress the warning.

How often does this bug happen?

Every time

System Info

No response

Additional Context

I'm not sure how easy it is (if possible at all!) to see the imports of the overriden MarkdownContent.astro, so maybe just an acknowledgement in the plugin config would suffice.

Thanks for your hard work with this plugin, it's awesome!

HiDeoo commented 2 months ago

Thanks for the feedback, glad you like the plugin :raised_hands:

This pattern to warn users regarding conflicting overrides is something that we started using since plugins were introduced in Starlight. Since then, I've gotten a few feedback regarding the ability to disable this warning and I've always been hesitant so far to do so but I think it may be time to reconsider this.

I see a few options here:

I'll still take some time to think about it and see if I can come up with another idea that could in a reliable way tell me if a component has been rendered or not, otherwise I'll probably go with the approach of adding an option to disable the warning (I assume this is what you meant with "an acknowledgement in the plugin config", right?).

HiDeoo commented 2 months ago

Ended up opening #19 as a draft that totally removes the warning.

After discussing with the topic with Chris (thanks you both for the valuable feedback) and also trying various approaches (Vite plugin, a middleware, etc which didn't provide the DX I was looking for or added too much complexity for 1 warning), I decided to go with the simplest approach that is to remove the warning.

The removal also includes a new documentation section that explains how to use the plugin with a custom MarkdownContent component with a complete example. I think this may ends up being more valuable (and linkable) than the warning itself which just used to say "render 'starlight-image-zoom/components/ImageZoom.astro'"…

I'll sleep on it before merging, but I think I want to try this approach in at least one plugin to see how it goes. Let me know if you have any feedback or concerns.

HiDeoo commented 2 months ago

Version 0.8.0 should now be available without the warning :tada: Let's see how it does on the long run and if I get many issues about it or not.

Thanks again for the issue and feedback 🙌