Docs/yaml json source client import #12634
Conversation
update gatsby remote
updating gatsby remote repo
update the repo removing 200+ commits
update after client search doc merge
Feb sync of the original repository
march update
wardpeet
added
type: documentation
| @@ -0,0 +1,184 @@ | |||
| --- | |||
|
|
|||
| ## title: " Client-side sourcing with JSON or YAML" | |||
There was a problem hiding this comment.
seems like small formatting issue?
should be
---
title: "Client-side sourcing with JSON or YAML"
---There was a problem hiding this comment.
Forgot this one... I'll add a entry to the toc linking each other in a bit
should those documents be also cross-linked?
|
|
||
| ## title: " Client-side sourcing with JSON or YAML" | ||
|
|
||
| # Table of Contents |
There was a problem hiding this comment.
those should level 2 header (##) - h1 should be document title (this is picked from frontmatter title in documention site)
There was a problem hiding this comment.
Ok. Something must have gone wrong with the linting job and it modified it to that. Looking at the my original document and then on gatsby looks like the following:
| ``` | ||
| |gatsby-client-sourcing-YAML-JSON | ||
| |content | ||
| - client-data.yaml / - client-data.json |
There was a problem hiding this comment.
@muescha the "/" is for an "or", because the document can go either way you can import JSON or YAML. If it makes it more clear for you i can change the "/" to an "or".
Transforming it into:
|gatsby-client-sourcing-YAML-JSON
|content
- client-data.yaml or - client-data.json
There was a problem hiding this comment.
Since you have later also the 2 js files without an "or" it is more clear to have here to have both files
when we assume the user do all the tutorial sections then he have both here
There was a problem hiding this comment.
I agree with @jonniebigodes that an or would be more clear than including both files on separate lines...it could confuse users to say they need both in this context even if they are both covered in this doc.
There was a problem hiding this comment.
@marcysutton then I would suggest to write the or because otherwise it looks like a malformed copy paste. For or is normally used the | in notations
| - client-data.yaml / - client-data.json | |
| - client-data.yaml or client-data.json |
Or better:
| - client-data.yaml / - client-data.json | |
| - client-data.yaml (or client-data.json) |
|
i have a general problem: you write this example is a client side rendering. but:
EDIT: |
|
Ok. But that's not "client" stuff. It is still rendered on server side. On client side it would import it when the page is imported and rendered on the client (client = browser) that means that I expect it loads the yaml file from server and then populate the data from it. |
|
@muescha what you've been commenting on here and what you pretend are two diferent things apparently. This pull request that derives from #11130. It's a simple way to import YAML or JSON directly into a page. And how to create a Gatsby website that is sourced from a YAML file, without the need of extra plugins, like it's equivalent in this docs page, nothing more, nothing less. For what you seek. a whole different approach would have to be taken, that appoach that is beyond the scope of this pull request. |
|
I see what you document and where it comes from. I like your article and how you shows to solve. But the many usages of the term "client" is wrong in this case. @pieh also noted this confusion in his code comment I like also to refer to this article, where is described a client data usage (client data = after a build and done on the client): https://www.gatsbyjs.org/docs/client-data-fetching/ |
|
@muescha regarding your last comment. The What @pieh mentioned was regarding for a better wording for the specific section in To avoid this discussion going back and forward, it's best to end it here and leave the @gatsbyjs/docs to go over the documents and example code. And with that issue a final ruling. If this needs to be redone i'll do it as per their instructions. |
There was a problem hiding this comment.
Hi there, thanks for working on this PR!
It's super helpful to show Gatsby users how to use JSON and YAML in the various ways you have here:
- importing directly into pages that already exist
- creating pages from YAML and JSON schemas
However, all of these examples are doing their magic at build time, not on the client-side. So for this PR to be accepted into the Gatsby docs it needs updated to reflect that, including all filenames, variables and passages of text. We want to be clear to users that this isn't happening on the client-side due to the way Gatsby works (a super great default in my opinion!).
| export default ClientYAML | ||
| ``` | ||
|
|
||
| As you can see, the code above is nothing more, nothing less than a functional stateless React component, that when rendered by Gatsby, and without any extra configuration will display a page sourced from a YAML file. |
There was a problem hiding this comment.
| As you can see, the code above is nothing more, nothing less than a functional stateless React component, that when rendered by Gatsby, and without any extra configuration will display a page sourced from a YAML file. | |
| The above code imports YAML source data and renders it in a functional stateless React component, without any extra configuration, to display a new Gatsby page. |
| @@ -0,0 +1,17 @@ | |||
| { | |||
| "title":"JSON used in the client with React and Gatsby", | |||
There was a problem hiding this comment.
This whole example needs to be updated to remove all mentions of "client" so as not to be misleading since the work is being done at build time.
| @@ -0,0 +1,10 @@ | |||
| import React from "react" | |||
There was a problem hiding this comment.
I don't think you need to include a 404 page in this example.
There was a problem hiding this comment.
404 is just added for consistency. My take on this is better leave it here to in order to avoid someone create a issue mentioning that the code does not match the template and subsequently create a pull request to amend it, putting additional work on you, and by you i mean the docs team, that it could be otherwise avoided. Also so that all audiences, ranging from people just getting in touch with Gatsby and Gatsby veterans can be confortable with.
| <h1>Sourcing from YAML</h1> | ||
| <div style={{ maxWidth: `960px`, margin: `1.45rem` }}> | ||
| <ul> | ||
| {[ |
There was a problem hiding this comment.
Why is this list here? Shouldn't the example be sourcing from a YAML file?
There was a problem hiding this comment.
This list is here because the example code holds both version and if a would be reader, wanted to pick up the code and see it working, if by any chance just opened http://localhost:8000 it would be presented with something, some content in there, also the list of endpoints that were created. That's my reasoning. Changing this is inconsequential
| import React from "react" | ||
| import uuid from "uuid" | ||
| import JSONData from "../../content/client-data.json" | ||
| const ClientJSON = () => ( |
There was a problem hiding this comment.
This is happening at build time, so it shouldn't have the word "Client" in it. The same goes for YAML.
per marcysutton feedback
|
@marcysutton just committed the changes you requested. Files were not linted, going to take this step by step. Address the changes first, if all is ok lint them and finally link the files in |
|
is there any interest in adding this document and example to the existing documentation? @marcysutton @gatsbyjs/docs |
|
There totally is! Sorry for the long wait :( I contacted Marcy so she'll have (hopefully) a final look on it and we can merge it. Thank you for your work, highly appreciated! |
…urce_client_import
|
sorry for the long wait. both docs were merged into a single one. Also the example aswell. |
|
Oops closed by mistake, sorry! @jonniebigodes Can you fix conflicts and merge master? That should get checks to be green again! (I tried to do so but don't think I have push access to your fork) |
sidharthachatterjee
removed
the
status: awaiting author response
|
@sidharthachatterjee i gave you access to the repo if you don't mind applying the changes so that we can move this along it would be awesome. |
|
@sidharthachatterjee thanks for the help on the conflicts. Greatly appreciated. |
There was a problem hiding this comment.
Thanks for this PR! A few high-level notes:
- This is going in the docs reference guides, so it's technically not a tutorial. I made a few suggestions to give it a consistent tone of voice and format.
- We can simplify some of the examples to not rely on uuid, which removes a dependency. Keeping things focused when possible eliminates variables of confusion and network dependencies.
Thanks again for working on this!
| title: Sourcing Content from JSON or YAML | ||
| --- | ||
|
|
||
| ## Table of Contents |
There was a problem hiding this comment.
We can remove the manual Table of Contents now, as we've got a TOC component! Woot!
There was a problem hiding this comment.
Is there some announcement or documentation about it?
PS: found only is PR #15251
|
|
||
| ## Introduction | ||
|
|
||
| As you come across Gatsby and start discovering the extent of it's possibilities, sometimes you might wonder about the basic things. |
There was a problem hiding this comment.
| As you come across Gatsby and start discovering the extent of it's possibilities, sometimes you might wonder about the basic things. | |
| As you work with Gatsby and discover the extent of its possibilities, you might have questions about sourcing data from a JSON or YAML file directly into a page or component. This guide will cover approaches for those techniques, as well as architecting a Gatsby site from a YAML file. |
We should combine the text into complete sentences, and avoid calling things "basic" since they might still be advanced topics for some learners. This is also a guide in the docs, not a tutorial.
|
|
||
| As you come across Gatsby and start discovering the extent of it's possibilities, sometimes you might wonder about the basic things. | ||
|
|
||
| Things like how to import a JSON file or a YAML file directly into a page or a component. |
There was a problem hiding this comment.
| Things like how to import a JSON file or a YAML file directly into a page or a component. |
Combined into the sentence above
examples/using-gatsby-with-json-yaml/src/templates/basicTemplate.js
Outdated
Show resolved
Hide resolved
examples/using-gatsby-with-json-yaml/src/templates/basicTemplate.js
Outdated
Show resolved
Hide resolved
examples/using-gatsby-with-json-yaml/src/templates/basicTemplate.js
Outdated
Show resolved
Hide resolved
examples/using-gatsby-with-json-yaml/src/templates/basicTemplate.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for this PR! A few high-level notes:
- This is going in the docs reference guides, so it's technically not a tutorial. I made a few suggestions to give it a consistent tone of voice and format.
- We can simplify some of the examples to not rely on uuid, which removes a dependency. Keeping things focused when possible eliminates variables of confusion and network dependencies.
Thanks again for working on this!
|
@marcysutton i believe i didn't miss anything. Also the example was updated accordingly. |
marcysutton
added
the
bot: merge on green
|
@marcysutton no need to thank. i should be the one thanking for the patience while going over this pull request. Going to wait until this is merged and delete the branch so that it does not clutter the repo. |
* rework performd on sourcing from yaml and client import for JSON or YAML * applied changes based on initial feedback * documents and example rebased. per marcysutton feedback * grammar and various fixes * docs merged and example rework * testing commit to check if frontmatter holds * doc-links updated per gillkyle comment * updated docs-links * update to latest version of docs-links.yaml * chore: fix document and example code and deps * code review feedback * clean up text based on page render, style
This pull request is continued from #11233 .
The original document was modified as per @marcysutton original input
It was split into two documents:
It's accompanied with the code for a fully functional example.
docs-links.yamlfile wasn't modified and committed with the changes mentioned by @marcysutton because during the linting process the following error was thrownNested mappings are not allowed in compact mappingsfor the file in question.Feel free to provide feedback.
Description
Related Issues
Addresses #11130