[1.0] Add basic Contentful source support #1084
Conversation
|
Deploy preview ready! Built with commit dcc986f |
|
Deploy preview failed. Built with commit dcc986f https://app.netlify.com/sites/gatsbygram/deploys/5931ed28a700c47395731b70 |
| parent: contentTypeItemId, | ||
| children: [], | ||
| name: entryItem.name, | ||
| ...entryItem.fields, |
There was a problem hiding this comment.
I am a little concerned about this. Someone's Contentful entries could have fields that conflict with standard Gatsby Node fields. Thoughts? Should this maybe be put under its own field e.g.
entryFields: entryItem.fields ?
Similar issue exists for assetNode. Today, there is no conflict but if Contentful's data model changes that could be an issue. It was already an issue for contentTypeNode.
There was a problem hiding this comment.
I think our only choice is to prefix any conflicting fields. And tell people when designing their schemas to avoid conflicting if possible with Gatsby's fields.
There was a problem hiding this comment.
OK, what do you think I should do here then? Just leave it? Sorry not sure who you mean by 'our' and 'people designing their schemas'. :)
There was a problem hiding this comment.
:-)
"our" === "us + anyone else who works on this plugin"
"people designing their schemas" === "users of this plugin who read the README"
What I meant was you'd loop over each field and if there's any field that conflicts with a top-level gatsby node field (id, children, parent, fields, internal) we change it e.g. to contentful_children https://www.gatsbyjs.org/docs/node-interface/
There was a problem hiding this comment.
I'm fine with doing this, but doesn't seem ideal. Doing it this way means the GraphQL interface for between Contentful Entries could be inconsistent from one to another because of a field naming collision. (Also, if in the future there are multiple plugins updating the same nodes, it's possible they can conflict with eachother?)
Maybe every plugin should have their own namespace which is prefixed to the field name. So in this case: contentful_productName
Definitely more verbose.. but would provide consistency going forward. I also accept it could be over-engineering it for now. Thoughts?
There was a problem hiding this comment.
Can you explain more what the fields field on Node is intended for?
// Reserved for plugins who wish to extend other nodes.
fields: Object,
That seems like a decent place to put all the Contentful specific fields. (But again could run into some collision issues if multiple plugins update the same nodes.. not sure if that is really a consideration at this point)
There was a problem hiding this comment.
Only one plugin is allowed to create a node of a given type so fields is an escape hatch to allow other plugins to add fields to another node without running into the risk of namespace collisions. Also with fields, again only one plugin can touch a given field.
So you only have to worry about fields from Contentful conflicting with the reserved Gatsby node fields. Which there isn't that many. So I think it'd be fine prefixing just conflicting fields (and perhaps logging a warning to encourage people to use a different name if possible).
|
Deploy preview ready! Built with commit dcc986f |
| // First create nodes for each of the entries of that content type | ||
| const entryNodes = entryList[i].items.map((entryItem) => { | ||
|
|
||
| const entryNode = { |
There was a problem hiding this comment.
I did not get to linking the entryNodes as described e.g.
brand___NODE: ID_OF_THE_PRODUCT
Not clear to me if I did that how to test it.
I left a note in the /gatsby-source-contentful/README.md about it as a future improvement, but can tackle it now with some more info.
There was a problem hiding this comment.
Check out https://github.com/gatsbyjs/gatsby/blob/1.0/packages/gatsby-source-drupal/src/gatsby-node.js#L89 and https://github.com/gatsbyjs/gatsby/blob/1.0/packages/gatsby-transformer-documentationjs/src/gatsby-node.js#L87
You could test it by saving some results from the Contentful as JSON and then running that through the transformation code.
|
This is looking awesome! There's been a few API changes but I'll take care of fixing those after it gets in. Glad you're working on this as I have a little Contentful side project I'll be starting on next week :-D |
| // Touch existing Contentful nodes so Gatsby doesn't garbage collect them. | ||
| _.values(store.getState().nodes) | ||
| .filter(n => n.internal.type.slice(0, 8) === typePrefix) | ||
| .forEach(n => touchNode(n.id)) |
There was a problem hiding this comment.
This isn't what we want while we're not using the sync API. Right now an entry could be deleted from Contentful and not be removed from the Gatsby site. Touching is only necessary if we retrieve from a remote API only the updated data so we "touch" the nodes which haven't updated but we know also weren't deleted. And oh lookie, there's even docs for this now :D https://www.gatsbyjs.org/docs/bound-action-creators/#touchNode
There was a problem hiding this comment.
Ah, makes sense. I'll update this and test by removing some entries in Contentful.
|
Nice work @mericsson . Really looking forward to this. |
|
Oh also please remove the yarn.lock and git ignore it. |
|
Good work @mericsson If you find the time, I think it would be super useful if you download the assets and insert them as File nodes, so they can be picked up by Sharp and you have control over image size and format from your code. Currently you can find the image url by id, but you have no control over it. Maybe this is outside of the scope of what you're trying to accomplish in this PR. |
|
@0x80 Contentful has an image api that could be used for this https://www.contentful.com/developers/docs/references/images-api/#/reference/image-manipulation I think for really large sites this would be preferable as otherwise each developer on a project would have to download all images for a site which again for larger sites would be considerable. I think @mericsson's plan is to expose the image API as a graphql schema. It'd be cool to have downloading images as an option though as it'd be more flexible + gatsby-like. |
| _.values(store.getState().nodes) | ||
| .filter(n => n.internal.type.slice(0, 8) === typePrefix) | ||
| .forEach(n => touchNode(n.id)) | ||
| .filter(n => n.internal.type.indexOf(typePrefix) === 0) |
There was a problem hiding this comment.
oh sorry this isn't needed actually either. Gatsby will "garbage collect" any node on startup that isn't either created or touched. Since you're creating (again) every node then we're fine. If something has been deleted on the Contentful side, than it just won't be created again which means Gatsby will kill it. So if we're not syncing the logic is pretty simple. Just create everything. When syncing then you have to worry about telling Gatsby what is and isn't still around.
|
New version of this has been committed. I went ahead and did all the Gatsby node referencing. I hadn't realized, but Contentful includes in its payload reference data for data referenced directly by any given entry. So for example, in Contentful if you specify that your Product entry type has a reference to Category, when getting the Product data via API, you will see the Category data inline in the response. To utilize Gatsby node linking, I deleted that data and just included the reference. Feels a little strange to undo that work that Contenful does for you, but also seems good to follow Gatsby node linking thoroughly. Contentful will not tell you about reverse links. So in the example above, it is Product referencing Category. Category will not tell you about Products that reference it. So I had to do a little bit of gymnastics to make those links. The finished product feels good and you can see some of the reverse linking in action on a category detail page. @0x80 Additional image support sounds interesting but outside of the scope of what I want to do in this PR. AFAIK, all the existing comments have been addressed. Let me know if you see issues w/ latest commits. Thanks! @KyleAMathews |
|
This looks fantastic @mericsson! Thanks for taking this on! |
|
@mericsson Awesome! @KyleAMathews don't know how I overlooked or forgot about their image api but I did 😄 |
|
Hiya @mericsson! 👋 This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here. Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! 💪💜 |
Includes:
/packages/gatsby-source-contentfulwhich is a prototype level new source plugin for pulling data from the Contentful.com CMS./examples/using-contentfulwhich is an example app leveraging the new plugin and an existing Contentful site. Site can be seen here: https://gatsby-using-contentful.netlify.com/.Lots of improvements are possible. Contentful seems like a great fit so I imagine this plugin will evolve tremendously.