florian-lefebvre / astro-integration-kit

A package that contains utilities to help you build Astro integrations.
https://astro-integration-kit.netlify.app
MIT License
46 stars 8 forks source link

feat!: make plugins work multi hooks #94

Closed florian-lefebvre closed 3 months ago

florian-lefebvre commented 3 months ago

Depends on #93, close #44

TODO:

netlify[bot] commented 3 months ago

Deploy Preview for astro-integration-kit ready!

Name Link
Latest commit 8c59e6f60953836bdca100fe1bffd4a330f20ad4
Latest deploy log https://app.netlify.com/sites/astro-integration-kit/deploys/660c3c3d9f466b0008a117e0
Deploy Preview https://deploy-preview-94--astro-integration-kit.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 3 months ago

Deploy Preview for astro-integration-kit ready!

Name Link
Latest commit dc239992c38ac6d7bd876d5c6cc6aa5c2de96488
Latest deploy log https://app.netlify.com/sites/astro-integration-kit/deploys/660e61037a890800082b6f96
Deploy Preview https://deploy-preview-94--astro-integration-kit.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

florian-lefebvre commented 3 months ago

@Fryuni would you be willing to clean the types at package/src/core/types.ts? I know you're good at TS and I've probably done dumb/inefficient stuff so your help would be much appreciated!

florian-lefebvre commented 3 months ago

@BryceRussell can you check if hasVitePluginPlugin still works as intended on the playground?

BryceRussell commented 3 months ago

hasVitePluginPlugin still works

florian-lefebvre commented 3 months ago

Awesome thanks for checking!

Fryuni commented 3 months ago

@Fryuni would you be willing to clean the types at package/src/core/types.ts? I know you're good at TS and I've probably done dumb/inefficient stuff so your help would be much appreciated!

Sure thing. I'll try to reserve some time for it later today or tomorrow

florian-lefebvre commented 3 months ago

Awesome thank you!

BryceRussell commented 3 months ago

Now that plugins can use multiple hooks, what do you think about adding hasVitePlugin to astro:config:done?

"astro:config:done": (params) => {
    return {
        hasVitePlugin: (plugin: string | PluginOption) =>
            hasVitePlugin(
                params as unknown as HookParameters<"astro:config:setup">,
                {
                    plugin,
                },
            ),
    };
},
florian-lefebvre commented 3 months ago

In addition to the current hook or as a replacement?

Fryuni commented 3 months ago

Now that plugins can use multiple hooks, what do you think about adding hasVitePlugin to astro:config:done?

I like that idea, but I think it should come with a very tiny little refactor to allow a utility to accept params from more than one hook and work the same. Worth an issue for a following PR

BryceRussell commented 3 months ago

In addition to the current hook or as a replacement?

In addition to the current hook

I like that idea, but I think it should come with a very tiny little refactor to allow a utility to accept params from more than one hook and work the same

Sounds good, it would be nicer to type this properly. I originally added this to test the changes in the playground and thought it could be an easy thing to support

florian-lefebvre commented 3 months ago

Sounds good! Let's track in a distinct issue