gatsby
gatsby copied to clipboard
CTF-next: Generate GraphQL schema based on Contentful Content Types
Discussion for the upcoming major release: https://github.com/gatsbyjs/gatsby/discussions/38585
Note for reviewers:
- Follow up to #12816 and will be a breaking, so major release.
- This branch will not be directly merged into master as some follow up PRs will come to add more features. See https://github.com/gatsbyjs/gatsby/issues/31385
- The schema.sql for the Contentful e2e test page has about 80 additions and over 300 deletions
This adds schema generation to the gatsby-source-contentful plugin. This will remove the most annoying restriction of the plugin: You now can have fields without a single instance in your content model and your GraphQL will no more fail because of this. See: https://www.gatsbyjs.com/plugins/gatsby-source-contentful/#restrictions-and-limitations
Goals
- [x] types will be generated based on the content model in Contentful. Your project will no more break when a field has no values at all.
- [x] Contentful Rich Text references will always be available and allow
__typenameandcontentfulon root level - [x] JSON fields are now real GraphQL JSON fields. This avoids the
There are conflicting field types in your data.warning and allows the JSON data structure to be changed in Contentful without breaking the Gatsby GraphQL Query - Create child nodes only when really necessary
- [x] RichText fields now use a simple object type
- [x] Location fields now use a simple object type
- [x] (Long) Text fields still use a node yet, as
gatsby-transformer-remarkrelies on it. Lets discuss later on.
- [x] Keep data from Contentfuls Common resource attributes within the
sysproperty (solved in #31007) - [x] Current state does break warm builds. I need to have a another look at reference resolver (solved in #31007)
- [x] Overall align the schema closer to the GraphQL Content API (first steps in #31007)
- Assets should have meta data like width on top level (#31115)
- Rich Text fields split up references in entries and assets (#31122)
- [x] Rename internal types with a scheme that avoids name collisions with content type names (#31286)
- [x] update e2e-test/contentful (first steps in #31007)
- ~update gatsby-remark-images-contentful to match new asset structure~ no changes needed
- [x] update gatsby-transformer-sqip to match new asset structure (as it is part of e2e-test/contentful)
- [x] the tags feature is now enabled by default
- [x] https://github.com/gatsbyjs/gatsby/pull/31395
- [x] The id generation is now done with two functions which slightly differ. This creates issues with references. WIP fix in https://github.com/gatsbyjs/gatsby/tree/feat/contentful-schema-generation-normalize-id-generation
Performance improvements
- [x] These changes will reduce the node count which will lead to improvements in bigger projects.
- [x] Rich Text fields no more need to
JSON.stringify()and rich text renderer no more needs toJSON.parse()
Upcoming goals in the next major / Follow up PRs:
See #31385
Migration steps / Breaking changes
See https://github.com/gatsbyjs/gatsby/issues/31385#issuecomment-839640682 for the full list of steps to migrate.
What do you think about adding a metadata: { ...assetItem.metadata }, to const assetNode?
Or should we make Tags a separate Node?
@itwasmattgregg I'd like to duplicate the pattern the Contentful GraphQL API implements this new feature.
To keep reviews doable, please keep in mind that this will be in a follow up PR
Decided to move #31395 out of this PR as we might need more time to get the information published through Contentful's CDA API.
This means I will try to finish #31286, get all CI green ✅ and we might do a first canary 🤔
That's awesome! Have been using gatsby and contentful for years and this has been a major blocking that made us switch back to gatsby-source-graphql.
Thanks guys. There is any way we can help to test or even contribute with this feature?
@lucashfreitas thanks, that schema issue also goes on my nerves for years already. I will be so happy when I add this version to my projects.
We should be soonish ready for first reviews and tests. I can mention you here as soon we are ready :)
Awesome @axe312ger . Please let me know when we ready for reviews - I have some large websites using this plugin and I am pretty keen to help and test it :)
Hey @axe312ger, thank you very much for this again :) Do you have any idea on when it will be ready to test? I am working on in a big website atm 1500 + pages that is using gatsby-source-graphql and I will migrate it over to gatsby-source-contentful once this feature is available to test. :)
@lucashfreitas we still clean up the codebase of the plugin to allow this change to be seamless as possible. So a few more weeks for sure. I badly hope for this summer, I want to use it in my personal projects as well 😅
Awesome, thanks @axe312ger . As soon as it's live we will update all of our websites to use it in favor of gatsby-source-graphl. :)
After we fixed some other critical issues with the plugin (especially asset downloads), I am now back to this!
The most relevant PRs are rebased now, left for this one:
- [x]
gatsby-node.jsunit test fails - looks like I broke some node creation / resolution - [x]
downloadLocalis broken
To simplify review, I created https://github.com/gatsbyjs/gatsby/pull/34032 as a sidecar PR. It does not contain any touches to the tests, that way reviewing the actual changes should be way easier :)
Even when CI disagrees. I got unit and e2e green on my machine :)
Hey @axe312ger, I've been keeping an eye on this mr for quite some time. The development seems to have come to a standstill, is this still being pursued?
I'm having to do manual schema customization more and more and I would love to have the "types will be generated based on the content model in Contentful" feature. Keep up the good work!
Before we can give this PR to review to the Gatsby Core team, the following things needs to be resolved:
- [x] Execute the https://gatsby.dev/node-convention-deprecation #37021
- [x] Finally rewrite to TypeScript
- [x]
sys.typeis the entity type (Entry, Asset), so it should not becomesys.contentType - [x] Reverse references should be restructured to use
linkedFromlike Contentful does - [x] https://github.com/gatsbyjs/gatsby/pull/30855#discussion_r850291289
- [x] https://github.com/gatsbyjs/gatsby/pull/30855#pullrequestreview-942074399
Also, resolve the following todos:
/gatsby-source-contentful/src/__fixtures__/rich-text-data.js
802,4: // @todo this fixture is unused
/gatsby-source-contentful/src/create-schema-customization.js
169,6: // @todo what do we do when preview is enabled? Emptry required fields are valid for Contentfuls CP-API
516,6: // @todo Is there a way to have this as string and let transformer-remark replace it with an object?
523,10: // @todo do we need a node interface here?
/gatsby-source-contentful/src/fetch.js
59,8: * @todo properly type this with TS
/gatsby-source-contentful/src/source-nodes.js
340,14: // @todo Detect reference fields based on schema. Should be easier to achieve in the upcoming version.
493,12: // @todo update the structure of tags
Open discussion points:
- Automatically turning all long text fields into MD(X) able text nodes is performance intense, especially in big projects. https://github.com/gatsbyjs/gatsby/pull/30855#discussion_r613426059
- The data schema of the Contentful Preview API differs from the Contentful Delivery API. E.G. some fields are not required. Should we just ignore the "required field value" flag in general? How does the Contentful GraphQL API deal with this?
@b-vb organizational blockers are resolved. Back on track now :)
Is it possible to get a status of this? It would make my day sooo much easier. The current implementation of schema generation is by far the most time consuming "feature" when working with Gatsby/Contentful :(
@samuel99 I know. It is a lot to do and we regularily have organizational blockers.
I won't give any estimation anymore, sorry.
We might get a alpha/beta as soon we get #37123 merged
As many are waiting here for some progress:
You consider yourself good in TypeScript?
I am happy about ANY feedback in #37123 !
Update:
Step by step, we are getting towards public alpha. Currently trying to please CI & e2e tests with the just merged TS rewrite.
Afterwards, I'll work on the bullet points in https://github.com/gatsbyjs/gatsby/pull/30855#issuecomment-1309013706.
In the meanwhile, the Gatsby team (WITHOUT exact date) is preparing a proper alpha/beta release process for this plugin so everyone can try and give feedback.
Last tasks to solve for first alpha
Hopefully complete list 😇
- [x] Finish and merge https://github.com/gatsbyjs/gatsby/pull/37811
- [x] Refactor in upcoming changes from https://github.com/gatsbyjs/gatsby/pull/37910
- [x] Double check that we run latest dependencies
- [x] Check if Cypress 12 does screenshots properly
- [x] Automatically turning all long text fields into MD(X) able text nodes is performance intense, especially in big projects. https://github.com/gatsbyjs/gatsby/pull/30855#discussion_r613426059
- [x] The data schema of the Contentful Preview API differs from the Contentful Delivery API. E.G. some fields are not required. Should we just ignore the "required field value" flag in general? How does the Contentful GraphQL API deal with this?
- [x] Implement Contentful JS SDK v10 with official TS support and remove our custom classes
- [ ] Bug: Warm Builds reset localFile to null on already existing nodes
- [x] Resolve all of the following ToDos:
./gatsby/packages/gatsby-source-contentful/src/__fixtures__/rich-text-data.js
- [x] 802,4: // TODO: this fixture is unused
./gatsby/packages/gatsby-source-contentful/src/types/contentful.ts
- [x] 60,6: // TODO: this field type might be defined by Gatsby already?
- [x] 62,6: // TODO: this field type might be defined by Gatsby already?
./gatsby/packages/gatsby-source-contentful/src/create-schema-customization.ts
- [x] 237,6: // TODO: what do we do when preview is enabled? Emptry required fields are valid for Contentfuls CP-API
- [x] 465,12: // TODO: fix the type for extraArgs in gatsby-plugin-iomage so we dont have to cast to any here
- [x] 639,8: // TODO: Is there a way to have this as string and let transformer-remark replace it with an object?
- [x] 646,12: // TODO: do we need a node interface here?
./gatsby/packages/gatsby-source-contentful/src/fetch.ts
- [x] 74,8: * TODO: properly type this with TS
./gatsby/packages/gatsby-source-contentful/src/normalize.ts
- [x] 111,4: // TODO: space id is actually not factored in here!
- [x] 685,12: // TODO:: how expensive is this?
./gatsby/packages/gatsby-source-contentful/src/source-nodes.ts
- [x] 310,12: // TODO: nodes of text fields should be deleted as well
- [x] 351,8: // TODO: mirror structure of Contentful GraphQL API, as it prevents field name overlaps
- [x] 519,10: // TODO add batching in gatsby-core
- [x] 566,14: // TODO: update the structure of tags
As CTF JS v10 is now released, with full typescript support, we can remove a lot of type definitions from this branch! :)
https://github.com/contentful/contentful.js/releases/tag/v10.0.0
As a reference for followers, still working on the bullet points in https://github.com/gatsbyjs/gatsby/pull/30855#issuecomment-1505013126
Mostly small things left :)
Feature we should add:
A debugflag, that lets all data loaded from CTF APIs stored to local file system. That way, if issues arise, customers can share their API responses easily with CTF support.
I am aware of one more bug, after fixing it, I'll test this version deeply with my personal Gatsby+Contentful projects.
Then we are ready for a public canary/alpha!
- [x] Bug: Warm Builds reset localFile to null on already existing nodes
Todos and bugs I found while testing with a real project:
- [x] useNameForId is not taken into account for reference fields
- [x]
metadatashould be calledcontentfulMetadata - [x]
linkedFromis missing from restricted field names - [ ] we should provide a way/tutorial on how to type JSON fields now
- [x] Progress bar is broken
- [x] codemods: node_locale needs to be transformed when filtering (all fields of sys field)
- [x] codemods: asset fields flattening when filtering does not work
- [x] codemods:
metadata->contentfulMetadata - [x] codemods: content type names should also be rewritten in fragments (
fragment Foo on ContentfulFooBar) - [x] codemods: content type names should also be rewritten in query extends (
... on ContentfulFooBar {) - [x] codemods: single value query filter does not transform id (
contentful_id->sys.id) - [x] codemods: sort is missing
- [x] codemods: would be great if we could rename ContentTypeNames also in the config of custom field resolvers (
createResolvers) - [x] codemods: warn users that have
createSchemaCustomizationimplemented -> their code is either outdated or no more needed - [x] codemods: flatten asset structure (
foo.file.details.width->foo.width) - ... this list will be extended ...
Next list of bugs I will fix before alpha:
- [x] sys.contentType is always null
- [x] fix foreignReferenceMap so big projects can store to gatsby cache
- [x] unions should not point to no more existing content types (validations in CTF can contain this data)
- [x] applying codemods on huge projects results in a OOM error (or requires 10GB RAM)
NODE_OPTIONS=--max_old_space_size=10240 npx jscodeshift -t ./packages/gatsby-codemods/transforms/gatsby-source-contentful.js src/**/*.ts src/**/*.tsx ./*.ts ./*.js
- [x] fix integration with
gatsby-plugin-page-creator - ... to be extended ...
We finally released a public alpha :)
Please try it out and give feedback in the new discussion -> https://github.com/gatsbyjs/gatsby/discussions/38585
Published new alpha version - [email protected]
Published new alpha version - [email protected]
👋 Long time plugin user here, nice progress!
Just to add context on the necessity of getting this PR merged: I started a discussion on the potential workarounds I need to do to get a site working that has a restricted content model ID, where I'm confident that this PR could solve my problems in a better way than the alternatives I can think of. https://github.com/gatsbyjs/gatsby/discussions/38908