withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
45.82k stars 2.41k forks source link

feat: Refactor integration hooks type #11304

Closed Fryuni closed 2 months ago

Fryuni commented 3 months ago

Changes

astro

As discussed on Discord, this breaks only a very narrow use case that wasn't documented as being supported and relied on the hooks not being extensible in order to create an extension workaround on top of it. So it is not being considered as a breaking change.

@astrojs/db

Testing

This doesn't change any runtime behavior

Docs

We can now document how integrations can add their own hooks 😄

Future scope

This PR only allows integrations to declare the types for their hooks; it doesn't provide any higher-level mechanism to trigger those hooks. Triggering them is already possible with the following code:

export default myIntegration: (): AstroIntegration => {
  let integrations: AstroIntegration[] = [];

  return {
    name: 'myLib',
    hooks: {
      'astro:config:setup': ({config}) => {
        integrations = config.integrations;
      },
      // Wherever some other logic runs
      'astro:build:done': () => {
        for (const integration of integrations) {
          integrations.hooks['myLib:eventHappened']?.(hookParameters);
        }
      },
    },
  };
}

In the future, we might provide some way for integrations to trigger their hooks with less boilerplate

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: 13e2244f7e53d32c56f4240861abb9ed3e9bd038

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Fryuni commented 2 months ago

Additionally, there is another change in here to @astrojs/db that will need a corresponding docs update, as this type being removed is currently documented.

It is not documented Sarah, AstroDbIntegration is nowhere in the docs. The defineDbIntegration utility is documented and that is why I kept it around.

sarah11918 commented 2 months ago

@Fryuni Sorry, I thought I checked closely, but I was concerned about this example which you're right, is not the type itself. As long as this example doesn't need updating, then no docs are needed, you're correct!

image

sarah11918 commented 2 months ago

Just pinging @Fryuni to look at my changeset suggestions! The Docs PR has already been approved and is ready for next week, so this is just tiny details now! :raised_hands: