Document how to extend integration hooks #8701
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
|
||
| ```ts | ||
| // your-integration/index.ts | ||
| import './types.ts'; |
There was a problem hiding this comment.
Realised we kind of skim over this import. Maybe should be mentioned in the introduction that this is needed?
There was a problem hiding this comment.
It's a bit complicated. It is not required at all; you just have to have some way for the consumer integration to load the type augmentation from the provider integration, and this makes it the most intuitive (by just importing the integration).
But there are multiple ways of doing it and IMHO none of them are intuitive for non-TS-power-users
There was a problem hiding this comment.
This is in the integration code itself though. If it’s not needed, maybe we remove it?
There was a problem hiding this comment.
Maybe I wasn't clear. Loading the type augmentation is required, but it isn't required to be done this way.
I'll explain this in the text
|
Hey @Fryuni , thanks for this addition! In general, I agree with all of Chris's edits here, but I'd like your confirmation before committing them to have as a base for me to start editing. Can you take a look over Chris's comments here and resolve them all before I do my (more text-based/structural) reviewing and possibly editing for language? Thanks! 🙌 |
[skip ci] Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
[skip ci] Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
There was a problem hiding this comment.
Hey @Fryuni , so excited to have this feature in docs!
I've left some comments/questions, some of which are absolutely coming from a lack of familiarity with authoring integrations, so I apologize for the hand-holding I'll need.
The main takeaways here are: I'd prefer a concrete example showing an actual use case, and I'm thinking that the page hierarchy might best change here.
I think there's a case to be made for this entire section being an <h2> and the whole thing is not about "defining custom hooks" but rather "creating and using custom hooks" (As I've mentioned in one comment, maybe that means that the existing "Hooks" would be improved as "Provided Hooks" or something like that)
Anyway, process this and see what of my comments make any sense! 😄
There was a problem hiding this comment.
I think this is looking pretty good?? In fact, when reviewing your PR initially, it kind of bothered me that we didn't say anything for Hooks, so this seems like a real improvement across the board!
Are you happy if this PR becomes just this reference documentation, or would you like to also try to fit in a recipe/guide in the same way we did for the Dev Toolbar API page?
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
I think the recipe/guide should go on a separate PR. It is not a requirement to get the feature released. |
| @@ -54,12 +54,18 @@ interface AstroIntegration { | |||
| logger: AstroIntegrationLogger; | |||
| }) => void | Promise<void>; | |||
| 'astro:build:done'?: (options: { dir: URL; routes: RouteData[]; logger: AstroIntegrationLogger; }) => void | Promise<void>; | |||
|
|
|||
| // ... integration-defined hooks gets added here | |||
There was a problem hiding this comment.
| // ... integration-defined hooks gets added here | |
| // ... any custom hooks from integrations |
I like this matching the language that's already on the page. Since most people won't have to worry about this, I like this looking like, "oh, and maybe some stuff will get added here" with a direct heading ("Custom hooks") they can go see what it means, if they're curious.
| }; | ||
| } | ||
| ``` | ||
|
|
||
| ## Hooks | ||
|
|
||
| Astro defines hooks that integrations can implement to execute during certain parts of Astro's lifecycle and to extend its functionality. The Astro hooks are defined in the `IntegrationHooks` interface, which is part of the global `Astro` namespace. |
There was a problem hiding this comment.
| Astro defines hooks that integrations can implement to execute during certain parts of Astro's lifecycle and to extend its functionality. The Astro hooks are defined in the `IntegrationHooks` interface, which is part of the global `Astro` namespace. | |
| Astro provides hooks that integrations can implement to execute during certain parts of Astro's lifecycle to extend their functionality. Astro hooks are defined in the `IntegrationHooks` interface, which is part of the global `Astro` namespace. |
I think the first sentence is nicer about the more general "providing/making available" and then the second sentence can talk about how they are technically "defined"!
Description