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

[gatsby-source-contentful] Problem with JSON contentTypes #2490

Closed
TomiTakussaari opened this issue Oct 17, 2017 · 14 comments
Closed

[gatsby-source-contentful] Problem with JSON contentTypes #2490

TomiTakussaari opened this issue Oct 17, 2017 · 14 comments

Comments

@TomiTakussaari
Copy link
Contributor

TomiTakussaari commented Oct 17, 2017

So I tried Contentful plugin with our space, and this happens:

Fetching default locale
⡀ source and transform nodesdefault local is : en-US
⠄ source and transform nodescontentTypes fetched 4
Updated entries  8
Deleted entries  0
Updated assets  0
Deleted assets  0
Fetch Contentful data: 2666.023ms
success source and transform nodes — 2.864 s
error UNHANDLED REJECTION

  Error: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "200" does not.

  - Array.forEach

  - Array.forEach

  - Array.forEach

  - Array.forEach

npm ERR! code ELIFECYCLE

Our contentful space contains contentType that has Swagger api-docs in JSON fields, and Swagger api-docs has fields that are named like "200", or "400", which apparently cause this problem. (Confirmed by disabling JSON field from response)

"responses": {
    "200": {
        "schema": {
            "$ref": "#/definitions/PaymentDemandResponse"
        },
        "description": "successful operation"
    },
    "400": {
        "schema": {
            "$ref": "#/definitions/ErrorResponse"
        },
        "description": "Invalid input"
    }

I am not that familiar with Gatsby's GraphQL layer, or contentful plugin, but it seems that it tries to make JSON field part of the GraphQL schema (?), and numbers are obviously invalid field names there.

Should it just use JSON fields as plain strings instead ?

@TomiTakussaari
Copy link
Contributor Author

So something like this:

          if (fieldType === `Object`) {
              entryItemFields[`${entryItemFieldKey}___NODE`] = createTextNode(entryNode, entryItemFieldKey, JSON.stringify(entryItemFields[entryItemFieldKey]), createNode);
              delete entryItemFields[entryItemFieldKey];
          }

next to this: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-contentful/src/normalize.js#L293

Consumers of this data would then have to use JSON.parse to actually use it as object.

Perhaps there is better way..

@KyleAMathews
Copy link
Contributor

hmmm strange, this code here is supposed to fix this problem

@TomiTakussaari
Copy link
Contributor Author

That only fixes fields named "id":

deepMap(object, (v, k) => (k === `id` ? fixId(v) : v))

My problem seems to be that Contentful entry contains JSON field that contains fields that have number as a name.

@TomiTakussaari
Copy link
Contributor Author

This reproduces the problem for me:

        {
            resolve: `gatsby-source-contentful`,
            options: {
                spaceId: `5mcft9q16uys`,
                accessToken: `e25be9e2b8266694d74e2709c63b436882fe9264240273c20a40dd1e4abdecbe`,
            },
        },
"gatsby": "1.9.70",
"gatsby-source-contentful": "^1.3.17",

screen shot 2017-10-17 at 20 58 27

@m4rrc0
Copy link
Contributor

m4rrc0 commented Oct 21, 2017

@TomiTakussaari I had a proposition similar to yours but instead of forcing JSON entries to be strings I added a "json" property to the object containing the whole object as string.
I guess in your case this would still be an issue because graphQL will still try to create the problematic keys.
Maybe your idea of importing objects as string is better. @Khaledgarbaya actualy agreed to that here #1703 and that is totally fine IMO too.

@m4rrc0
Copy link
Contributor

m4rrc0 commented Feb 25, 2018

Isn't this solved by PR #2967 ?

@Khaledgarbaya
Copy link
Contributor

Isn't this solved by PR #2967?
That PR only adds a stringified version of the JSON to the node.

Tha problem happens when you have JSON with numbers as keys or anything that does not pass /^[_a-zA-Z][_a-zA-Z0-9]*$/ test, this a GraphQL requirement.
Being able to query the JSON object using GraphQL is nice but I think it won't work for all case because we can't control what you a user writes there.

I can see few options to solve this.

  • the source plugin detect these invalid fields and prepend something, which I am not a big fan of altering the data
  • make the JSON queryable option optional, you will have always the stringified version always.
  • Make the JSON only available as a string.

I'll be happy to create a Proposal issue as this might be a breaking change depends on the option.

@KyleAMathews @MarcCoet @TomiTakussaari @sarahatwork

@fk fk added the API/Plugins label Feb 26, 2018
@TomiTakussaari
Copy link
Contributor Author

For me, making JSON field only available as a string makes most sense.

Having "JSON queryable" option would mean that if option is enabled, its possible to break site deploy by just adding wrong kind of JSON to any JSON field in Contentful space.
So if option approach is chosen, it should IMO be a whitelist of ContentTypes.

Also I can't even think of a use case where I would like have my JSON field data queryable when using Contentful, I feel like its not proper way to use Contentful.
JSON field data should just be data to be used somewhere. There could be other opinions though :)

@Khaledgarbaya
Copy link
Contributor

I am also for keeping just the text content of the JSON.

@KyleAMathews
Copy link
Contributor

Yeah, that would simplify things if people only could query the JSON string which they JSON.parse client side.

@m4rrc0
Copy link
Contributor

m4rrc0 commented Feb 27, 2018

Alright. I see. JSON as string is perfect for me too. I only use it like that anyway.

@Khaledgarbaya
Copy link
Contributor

ok cool I am a bit under the weather these days, once I am feeling better i will create a PR, should be in the next week.

@KyleAMathews
Copy link
Contributor

Due to the high volume of issues, we're closing out older ones without recent activity. Please open a new issue if you need help!

@m4rrc0
Copy link
Contributor

m4rrc0 commented Aug 17, 2019

Apparently this still hasn't been implemented...
JSON nodes still get populated with what is inside the JSON field.

Surely this must be easy to work around with the new schema customization API but I get a node validation error that won't go away. I tried a few things similar to:

exports.createSchemaCustomization = ({ actions, schema }) => {
  const { createTypes } = actions
  const typeDefs = [
    `
    type contentfulBlablaJsonNode implements Node @dontInfer {
      id: ID!
    }
    `,
  ]
  createTypes(typeDefs)
}

I guess validation happens before the createSchemaCustomization hook. Is there a way to modify the node before it gets validated?

sherwinski referenced this issue in imgix/contentful Jan 21, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[1](https://github.com/gatsbyjs/gatsby/issues/2490)[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3) stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 21, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 31, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 31, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 31, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 31, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
sherwinski pushed a commit to imgix/contentful that referenced this issue Jan 31, 2022
Because the keys of nested JSON fields (e.g. `tags`, `custom_fields`,
    `colors`) can vary from one object to the next, this can cause
brittleness for users who are interfacing with the Contentful API using
GraphQL. This is because GraphQL requires users to be explicit about
what fields they want to return, rather than being able to query an
object for all fields regardless of what they are.

To help with this, some users have suggested
[[1](gatsbyjs/gatsby#2490)] [[2](https://forums.fauna.com/t/how-to-store-arbitrary-json-object-via-graphql/142/3)] stringifying nested JSON fields and then parsing it clientside. This better ensures that queries won't break even if the returning data's underlying shape somehow breaks. Now each of the following fields will be returned as a string:

- tags
- custom_fields
- colors.dominant_colors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants