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.

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: add new doc on why Gatsby uses GraphQL #11787

Merged
merged 15 commits into from
Mar 8, 2019

Conversation

jlengstorf
Copy link
Contributor

This is a new doc that walks through the process of creating increasingly more complex pages in an attempt to better illustrate why GraphQL is so useful in Gatsby.

It starts with hard-coded components, moves on to context, and ends with complex pages that have images — after showing the “hard way” of passing everything through context and still ending up with unoptimized images, etc., it then shows how GraphQL can solve for many complexity issues that would otherwise be really hard to manage as a site scales.

I wanted to get initial feedback on this now, but the plan is to add video lessons to this that will walk through building these pages.

@marcysutton @KyleAMathews @gatsbyjs/core @shannonbux @amberleyromo if I missed anything or otherwise need to revise the code, please let me know so I can address it before I make videos. 😅

Thanks!

@wardpeet wardpeet added the type: documentation An issue or pull request for improving or updating Gatsby's documentation label Feb 15, 2019
Copy link
Contributor

@DSchau DSchau left a comment

Choose a reason for hiding this comment

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

Looking great! Left some comments--feel free to ignore or tweak to your heart's desire 🎉

docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Show resolved Hide resolved
docs/docs/why-gatsby-uses-graphql.md Outdated Show resolved Hide resolved
Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

Nice!

Like the build up of progressively more complex examples.


The `context` property accepts an object, and we can pass in any data we want the page to be able to access.

> **NOTE:** There are a few reserved names that _cannot_ be used in `context`. They are: `path`, `matchPath`, `component`, `componentChunkName`, `pluginCreator___NODE`, and `pluginCreatorId`.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we document this anywhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I'm aware of — the docs are pretty light on information about context in general. @pieh pointed me to the source code, which is where I grabbed these. https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/redux/actions.js#L134-L141

Copy link
Contributor

Choose a reason for hiding this comment

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

It could we good to document those elsewhere, it would work well on this page: https://www.gatsbyjs.org/docs/creating-and-modifying-pages/#pass-context-to-pages

I'll create an issue and PR for it.

Copy link
Contributor

@shannonbux shannonbux left a comment

Choose a reason for hiding this comment

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

This seems really well-written. Only suggestion I have is to have a ToC at the top. I know the doc isn't long, but each header is clear and they are all equal to each other, so I think it'd be cool to see an overview at the top. Although I suppose that could go against the purpose of the doc, which is a structured argument that needs to be read through completely to be understood. Is that accurate?

@jlengstorf
Copy link
Contributor Author

@shannonbux I don't know if I have a strong opinion either way on that. I do think this is designed to make a structured argument, and I wouldn't want the ToC to imply that we're necessarily calling some of these examples "ready-to-use code samples". But it's also nice to be able to navigate between sections.

I think I'd really just like to get the tutorial-style sidebar nav working on all docs pages, personally. 😄

@calcsam
Copy link
Contributor

calcsam commented Feb 19, 2019

@shannonbux it would probably make more sense to have a TOC generated programmatically from headings for all docs pages rather than individually on a per-page basis.

@shannonbux
Copy link
Contributor

@jlengstorf makes sense to me, and @calcsam that could be cool as long as perhaps there is an opt-out ability for really short docs (like ones that fit on a page), because in that case, the ToC might take more room than its worth. I don't think it's a top priority yet.

@marcysutton
Copy link
Contributor

a TOC generated programmatically from headings for all docs pages

That sounds like a great reusable component! I know I would have used it in the new contributing section (where there are some hard-coded). Opt-in would be the easiest transition, in my opinion, so we could play around with heuristics before automatically inserting a TOC into every docs page (n headings? Doc text length? Include subheadings?). I can file an issue for a reusable component so we can keep this post moving.

@calcsam
Copy link
Contributor

calcsam commented Feb 20, 2019

@marcysutton optimizing heuristics and an opt-in approach seems like a great idea!

Check out https://www.gatsbyjs.org/packages/gatsby-transformer-remark/#getting-table-of-contents if you haven't seen it already --

@calcsam
Copy link
Contributor

calcsam commented Feb 20, 2019

Also we were talking about this before in this issue: #6896

@KyleAMathews
Copy link
Contributor

It's pretty common to support a short code like [toc] to add a table of contents at that point. Agree that always adding table of contents isn't what we want as they're only useful on long docs. Though yeah, we could also add a heuristic that e.g there's 1500+ words and > N headers.

@jlengstorf
Copy link
Contributor Author

Waiting on the egghead videos to be published. Once they’re live this will be good to merge.

@jlengstorf jlengstorf marked this pull request as ready for review March 7, 2019 21:30
@jlengstorf jlengstorf requested a review from a team March 7, 2019 21:30
@jlengstorf
Copy link
Contributor Author

This is ready! Can I get a final review/merge? cc @marcysutton

Copy link
Contributor

@marcysutton marcysutton left a comment

Choose a reason for hiding this comment

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

This is a fantastic doc! My only question is where it will live in the nav. Do you need to update doc-links.yml?

@jlengstorf
Copy link
Contributor Author

@marcysutton ha. Yes. 😅 Let me do that really quickly.

@marcysutton
Copy link
Contributor

Perhaps you could link to your new doc from here too? https://www.gatsbyjs.org/docs/querying-with-graphql/#why-is-graphql-so-cool

@jlengstorf jlengstorf requested a review from a team as a code owner March 8, 2019 23:20
@jlengstorf
Copy link
Contributor Author

@marcysutton added a few links + updated the sidebar. I think we're good to go?

Copy link
Contributor

@marcysutton marcysutton left a comment

Choose a reason for hiding this comment

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

There are a few links without trailing slashes, but that's a very minor issue. Great work @jlengstorf!

@jlengstorf jlengstorf merged commit b645b25 into gatsbyjs:master Mar 8, 2019
@jlengstorf jlengstorf deleted the docs/why-gql branch March 8, 2019 23:28
calcsam pushed a commit that referenced this pull request Mar 9, 2019
<!--
  Have any questions? Check out the contributing docs at https://gatsby.dev/contribute, or
  ask in this Pull Request and a Gatsby maintainer will be happy to help :)
-->

## Description

Documenting limitations with React context, as per the feedback/comments in #11787.

Closes #12423
@muescha
Copy link
Contributor

muescha commented Mar 10, 2019

Section: Add the necessary plugins to load data into GraphQL

In the egghead video you use yarn to add the plugins and yarn develop instead of gatsby develop

In the doc npm for install, but later yarn develop

It should always use one of it and also doc and video should be equal

We should stick for the rule: npm for user of Gatsby

@muescha
Copy link
Contributor

muescha commented Mar 10, 2019

A site node: it takes me a lot of time to find out what to use and when: npm or yarn

npm as user of Gatsby and yarn as developer?

Suggestion: There should be also a doc page about the differences and when to use what

@muescha
Copy link
Contributor

muescha commented Mar 10, 2019

Section: Add the necessary plugins to load data into GraphQL

The video contains more info than the text of this section. The text ends with only slug in query And the rest is missing as text before the next video

@LekoArts
Copy link
Contributor

Suggestion: There should be also a doc page about the differences and when to use what

I think that’s out of scope of the Gatsby docs. One could google that to find the differences. And you can use both anytime, there are no „rules“ for when to use what :)

@muescha
Copy link
Contributor

muescha commented Mar 10, 2019

I mean in context of gatsby. If you new to npm then it is a complicated to decide.

But for example: if i use yarn as a user then I never can do gatsby develop because this uses npm

@LekoArts
Copy link
Contributor

If you’re a npm user then you don’t have to switch except when you want to contribute to the gatsby monorepo (as it uses yarn workspaces). So only displaying npm commands everywhere is fine as it’s interchangeable with yarn.

No, you misunderstand something there. gatsby develop is available if you installed the Gatsby CLI. yarn develop and npm run develop are available if the package.json has a script called develop.

@muescha
Copy link
Contributor

muescha commented Mar 10, 2019

maybe we extract this npm/yarn discussion into a new issue? how we can move comments?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants