Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(docs): split GraphQL image fragments APIs to their own pages #21536

Closed
wants to merge 3 commits into from
+36 −81
Closed

(docs): split GraphQL image fragments APIs to their own pages #21536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f94d73f
split up graphql api fragments
tesseralis Feb 17, 2020
e695117
stopgap to display the fragments
tesseralis Feb 17, 2020
61d8e69
delete static query file
tesseralis Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/docs/contentful-fragments.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
title: Contentful Fragments
jsdoc: ["gatsby-source-contentful/src/fragments.js"]
contentsHeading: Fragments
includeTopLevel: true
---

The following fragments are available in any site with `gatsby-source-contentful` installed and included in your `gatsby-config.js`. These fragments generally mirror the fragments outlined in the `gatsby-transformer-sharp` package.
29 changes: 3 additions & 26 deletions docs/docs/graphql-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,33 +231,10 @@ For more information, check out the docs guide on [using fragments in Gatsby](/d

Some fragments come included in Gatsby plugins, such as fragments for returning optimized image data in various formats with `gatsby-image` and `gatsby-transformer-sharp`, or data fragments with `gatsby-source-contentful`.

#### Image sharp fragments
Documentation on these fragments can be found in these pages:

The following fragments are available in any site with `gatsby-transformer-sharp` installed and included in your `gatsby-config.js`.

Information on querying with these fragments is also listed in-depth in the [Gatsby image API docs](/docs/gatsby-image/), including options like resizing and recoloring.

<GraphqlApiQuery>
{data => (
<APIReference
relativeFilePath={data.transformerSharp.nodes[0].relativePath}
docs={data.transformerSharp.nodes[0].childrenDocumentationJs}
/>
)}
</GraphqlApiQuery>

#### Contentful fragments

The following fragments are available in any site with `gatsby-source-contentful` installed and included in your `gatsby-config.js`. These fragments generally mirror the fragments outlined in the `gatsby-transformer-sharp` package.

<GraphqlApiQuery>
{data => (
<APIReference
relativeFilePath={data.contentfulFragments.nodes[0].relativePath}
docs={data.contentfulFragments.nodes[0].childrenDocumentationJs}
/>
)}
</GraphqlApiQuery>
- [Image Sharp Fragments](/docs/image-sharp-fragments/)
- [Contentful Fragments](/docs/contentful-fragments/)

**Note**: the above fragments are from officially maintained Gatsby starters; other plugins like `gatsby-source-datocms` and `gatsby-source-sanity` ship with fragments of their own. A list of those fragments can be found in the [`gatsby-image` README](/packages/gatsby-image#fragments).

Expand Down
10 changes: 10 additions & 0 deletions docs/docs/image-sharp-fragments.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
title: Image Sharp Fragments
jsdoc: ["gatsby-transformer-sharp/src/fragments.js"]
contentsHeading: Fragments
includeTopLevel: true
---

The following fragments are available in any site with `gatsby-transformer-sharp` installed and included in your `gatsby-config.js`.

Information on querying with these fragments is also listed in-depth in the [Gatsby image API docs](/docs/gatsby-image/), including options like resizing and recoloring.
52 changes: 0 additions & 52 deletions www/src/components/api-reference/doc-static-queries.js
View file
Open in desktop

This file was deleted.

5 changes: 5 additions & 0 deletions www/src/data/sidebars/doc-links.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,11 @@
link: /docs/actions/
- title: GraphQL API
link: /docs/graphql-api/
items:
- title: Image Sharp Fragments
link: /docs/image-sharp-fragments/
- title: Contentful Fragments
link: /docs/contentful-fragments/
- title: Gatsby Node APIs
link: /docs/node-apis/
- title: Gatsby Node Helpers
Expand Down
13 changes: 10 additions & 3 deletions www/src/templates/template-api-markdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ const normalizeGatsbyApiCall = array =>
return { name: entry.name, codeLocation }
})

const mergeFunctions = (data, context) => {
const mergeFunctions = (data, context, includeTopLevel) => {
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the question about the meaning of includeTopLevel... seems like we should also put a note about that in the contributing docs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤷‍♂

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid points. I think your suggestions for where to add the information make sense and I'm comfortable with keeping the list of fragments.

)
.map(def => {
if (!context.apiCalls) {
// When an api call list is not available, the line numbers from jsdoc
Expand Down Expand Up @@ -81,7 +83,11 @@ export default function APITemplate({ data, location, pageContext }) {
const page = data.mdx

// Cleanup graphql data for usage with API rendering components
const mergedFuncs = mergeFunctions(data, pageContext)
const mergedFuncs = mergeFunctions(
data,
pageContext,
page.frontmatter.includeTopLevel
)
const description = page.frontmatter.description || page.excerpt

return (
Expand Down Expand Up @@ -159,6 +165,7 @@ export const pageQuery = graphql`
description
contentsHeading
showTopLevelSignatures
includeTopLevel
}
...MarkdownPageFooterMdx
}
Expand Down