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

WIP: fix(gatsby-source-contentful) No Contentful assets causes build failures #21693

Closed

Conversation

joshuagagne
Copy link
Contributor

Description

Inlines an image to guarantee build has an image to infer the model around.

Not the biggest fan of this solution.

Also tried to get a schema typed before hand to guarantee the schema types existed, but that runs into problems where multiple schema types of the same name exist.

This does not actually make the fragments pass when executed on inside a query, as this mock item doesn't actually exist in Contentful the resolved urls are not accurate. I toyed with the idea of actually having a local gatsby image file to reference if no assets are found.

Related Issues

Fixes #16455
Fixes #15397

@joshuagagne joshuagagne requested a review from a team as a code owner February 23, 2020 22:58
@pieh
Copy link
Contributor

pieh commented Feb 24, 2020

Also tried to get a schema typed before hand to guarantee the schema types existed, but that runs into problems where multiple schema types of the same name exist.

You mean when user defines the type? IMO this should be the way to go here instead of creating "mock" asset node, that might mean making fixes (or changes) in gatsby core to fix the issue you are talking about. Can you clarify what kind of issues you saw?

@joshuagagne
Copy link
Contributor Author

Well specifically we can't add it with the createSchemaCustomization's createType API as that results in the type being defined twice when content does exist.

onPreExtractQueries where we used to filter out these fragments and check for cache existence could be a useful spot to check things. But the side effect to fix this would have to be moving / deleting the fragments.js files as the query-compiler.js finds it and pulls the fragements out. I'm unfamiliar with the fragment parsing so I didn't go down this route, but you could put a function inside fragements.js that could be executed to get a boolean on whether to skip processing of that file. Seems inelegant.

In sourceNodes providing the default asset allows us to get into normalize.js's createAssetNodes function and create a node. Looks like this function is responsible for creating the ContentfulAsset type.

@joshuagagne
Copy link
Contributor Author

joshuagagne commented Feb 25, 2020

I updated the code's default asset creation to instead call createNode directly. On the positive side this looks much cleaner, on the down side it causes a warning to be logged because I didn't actually add an item.

warn The asset with id: defaultMediaItem, contains no file.

Still, at least this would allow a user with a brand new contentful space to have a build pass immediately after adding the plugin. 😖 course now the starter tests failed.

Is there a spot after this that would create schema definitions? createNode doesn't seem like it should.

@vladar
Copy link
Contributor

vladar commented Mar 4, 2020

Well specifically we can't add it with the createSchemaCustomization's createType API as that results in the type being defined twice when content does exist.

This sounds wrong. Can you clarify a bit what do you mean by this? Do you get an error or some kind of warning?

One of the reasons why schema customization API was added in the first place is to fix this issue with missing content. But it shouldn't break at all when content exists.

@joshuagagne
Copy link
Contributor Author

If you attempt to create the type in the createSchemaCustomization stage it results in the error:

UNHANDLED REJECTION Schema must contain uniquely named types but contains multiple types named "ContentfulFixed".

As far as I can tell defining the adding the type early clashes with the createNode later on. I was hoping it'd recognize the type and merge the schemas.

An alternate fix seems to be to just remove the graphQL rule that validates the fragments. The fragments get created, and if used in a query will cause the same error message to be thrown, it'll just happen at run time instead of build time.

Would I be correct that the unfortunate side effect of removing this rule is that query-compiler.js is not limited to only fragments but all user defined queries? So a custom graphQL query in a page / component that should fail because of a typeName miss match would not.

@pvdz
Copy link
Contributor

pvdz commented Apr 16, 2020

Hi @joshuagagne . It looks like this PR is still a wip (as also suggested by the title). I'm going to put it in Draft mode. This won't change much for you. You can pull it out of draft mode at any time when you feel the PR is ready and people that were involved in the PR will still see the updates and everything. Thanks!

@pvdz pvdz marked this pull request as draft April 16, 2020 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants