Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-7083--docusaurus-2.netlify.app/ |
|
Size Change: +56 B (0%) Total Size: 805 kB
ℹ️ View Unchanged
|
Josh-Cena
added
the
status: awaiting review
slorber
left a comment
slorber
left a comment
There was a problem hiding this comment.
Mitigated feelings about this.
Is it an API design that users would easily understand?
| pluginName: string, | ||
| ): GlobalData[string]; | ||
| options?: UseDataOptions, | ||
| ): GlobalData[string] | undefined; |
There was a problem hiding this comment.
I guess it could be possible to use TS overloads to avoid TS users to need to use ! despite the failfast option?
There was a problem hiding this comment.
Technically yes, but... err... at least not quite useful for us
| useAllPluginInstancesData(pluginName: string) | ||
| function useAllPluginInstancesData( | ||
| pluginName: string, | ||
| options?: {failfast?: boolean}, |
There was a problem hiding this comment.
is it really an option that we want to expose publicly? 🤷♂️
| usePluginData('docusaurus-plugin-content-docs', pluginId) as GlobalPluginData; | ||
| usePluginData('docusaurus-plugin-content-docs', pluginId, { | ||
| failfast: true, | ||
| }) as GlobalPluginData; |
There was a problem hiding this comment.
probably not needed with TS overloads
There was a problem hiding this comment.
It's needed because we are casting it to the shape of doc plugin's global data instead of the generic unknown global data. Theoretically we need to validate but in spirit of pragmatism and bundle size I didn't
|
So the point is that in the only place where we are using global data, we are using it in a fail-safe way anyways. I think most users would also appreciate a more fail-safe API and handle |
|
ok 👍 good enough |
Josh-Cena
removed
the
status: awaiting review
3 participants
Motivation
The docs plugin hook can't use fail-fast because "it's used on non-doc pages" (but I think that's probably not the case anymore). But anyways, to avoid refactor hazards I just added a
failfastoption so we can take advantage of theuseAllPluginInstancesDatahook.Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Build