(docs): split GraphQL image fragments APIs to their own pages #21536
Conversation
| const normalized = normalizeGatsbyApiCall(data.nodeAPIs.group) | ||
|
|
||
| const docs = data.jsdoc.nodes.reduce((acc, node) => { | ||
| const doc = node.childrenDocumentationJs | ||
| .filter(def => def.kind !== `typedef` && def.memberof) | ||
| .filter( | ||
| def => def.kind !== `typedef` && (includeTopLevel || def.memberof) |
There was a problem hiding this comment.
adding includeTopLevel is a hack for now. Once we're approved that we want to do this, I can figure out a less hacky solution.
There was a problem hiding this comment.
Would you mind expanding on that? I'm not too sure what the name suggests. And is the question about approval about adding that to the frontmatter or the general approach?
There was a problem hiding this comment.
Originally, we're filtering the docs based on if def.memberof is defined -- i.e. only using docs that are part of some object, like actions.createPage.
Since the fragments are exported by themselves they'll be filtered out if we don't change this query in some way. My current solution is to add an option in the frontmatter to disable this check.
But I think the better solution would be to figure out why this query was needed (as in, what things we don't want to include in the doc) and come up with a better check that works for both that and the new fragments query.
There was a problem hiding this comment.
To my knowledge, this change is good to go. But do we have any additional clarity on this? @LekoArts, do you know why the query is there or whether this might cause unforeseen issues?
There was a problem hiding this comment.
As for the question about the meaning of includeTopLevel... seems like we should also put a note about that in the contributing docs.
There was a problem hiding this comment.
Looking at the Git history, I believe @gillkyle would know more about that query as it appears he wrote the file. It seems worth getting to the bottom of it rather than doing a workaround with contributing docs, in my opinion.
There was a problem hiding this comment.
The idea of that query was to tie the updates to the fragments file in the core codebase to the docs so they wouldn't fall out of sync. This was a different use case than the other API reference pages so I see why there would need to be a bit of a change to the logic of the template.
As it stands now it's not doing much more than list all the exported fragments so if it would simplify things to just make that a list that we occasionally have to update from time to time I don't think that would be a problem either. Seems like maybe the added complexity of querying the list of fragments might not be worth whatever small benefit it adds 🤷♂
There was a problem hiding this comment.
That's more or less where I'm at, Kyle. I'm struggling to think of a different way to do this that wouldn't create additional opportunities for docs to get out of sync. Also, my point about the contributing docs stands regardless of what we do if we go for something that requires setting an option somewhere, even if it's not exactly this.
There was a problem hiding this comment.
Agreed in just making it a static list. There's precedence for it: many of our docs don't rely on pulling in JSDoc. In fact, we already have a list of the ImageSharp fragments in the Gatsby Image API, so an argument could be made that this content is repeated. I'm not sure what the best place would be to put information about the Contentful fragments though -- maybe in the gatsby-source-contentful readme and "sourcing from contentful", with a link to the Gatsby Image API for more details on the fragment options.
There was a problem hiding this comment.
Solid points. I think your suggestions for where to add the information make sense and I'm comfortable with keeping the list of fragments.
Description
Move graphql image fragment API documentation to their own pages, taking advantage of our API template to decrease build size by 10%.
Related Issues
Fixes: #21109