Skip to content

Conversation

@Convly
Copy link
Member

@Convly Convly commented May 24, 2021

GraphQL API v4
This PR introduces a draft version of the GRAPHQL API in v4.

We want to get the conversation started around this topic to get feedbacks quickly so feel free to ask questions and give ideas :)

You can read it here

@Convly Convly added the v4 label May 24, 2021
@Convly Convly requested a review from alexandrebodin May 24, 2021 09:42
@strapi-cla
Copy link

strapi-cla commented May 24, 2021

CLA assistant check
All committers have signed the CLA.

@alexandrebodin alexandrebodin marked this pull request as draft May 24, 2021 09:42
@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/new-version-questions-thoughts-v4/4824/1

Co-authored-by: Gustav Pursche <pursche@posteo.de>
@gu-stav
Copy link
Contributor

gu-stav commented May 25, 2021

Thanks for posting this. I do have a couple remarks / questions:

  • publicationState do you think it might make sense to make this a string, so in some unknown future, a content type could follow a custom flow, which has more states than "live" and "draft"? Also it could be nice to use this through the filtering system, to be consistent:
{
  documents(publicationState: { $eq: "live" }) {}
}
  • I'm not sure about the naming of pageSize. It took me while to understand what could be the difference to total and pageCount. I'd propose: totalPages, currentPage, itemsPerPage instead. Gatsby used skip and `limit, which is very concise and clear to me.

  • In general I'd be very happy if there could be a way (maybe based on the query) to disable pagination at all, because if you need to query for all items I guess it's more resource intensive to retrieve them page by page. Maybe this could look like documents(pagination: FALSE) {}. I do think though, having it enabled by default makes sense. This would also improve the following scenario:

    • Static page generator fetches items of page 1
    • Editor saves something in the CMS at the same time
    • Static page generator fetches items of page 2, which now has an offset by one

This could also be solved, if the pagination wasn't done by an index, but by a hash, so the content of a page doesn't change.

  • Personally I don't like the syntax of sory much and do think it would make more sense to stick to ASC and DESC, eg.: documents(sort: "title:asc") or documents(sort: ["title:asc", "price:desc"]). I also do like the way Gatsby does it more:
{
  documents(
    sort: {
      fields: [title]
      order: ASC
    }
  ) {}
}

To support more than one dimension it could look like:

{
  documents(
    sort: {
      fields: [title, publidationDate]
      order: [ASC, DESC]
    }
  ) {}
}
  • In the filtering section you mention this example: title: { $startsWith: "Book" }. That reminds me, that it would be super useful, if we could introduce custom filters, to e.g. compare dates, or geo-coded data (thinking about around, closeTo ...)

@Stun3R
Copy link

Stun3R commented May 25, 2021

You didn't talked about the schema.json. Will you make some change as it's autogenerated ? (Working on the SDK to generate types based on the schema.json you provide)


### Pagination

**Parameter** `pagination`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add limit -1 or equivalent

@alexandrebodin
Copy link
Member

You didn't talked about the schema.json. Will you make some change as it's autogenerated ? (Working on the SDK to generate types based on the schema.json you provide)

We are going to continue export the schema but it will jsut have the new format :) Anything specific you had in mind ?

@Stun3R
Copy link

Stun3R commented Jun 2, 2021

You didn't talked about the schema.json. Will you make some change as it's autogenerated ? (Working on the SDK to generate types based on the schema.json you provide)

We are going to continue export the schema but it will jsut have the new format :) Anything specific you had in mind ?

It would be great if you could separate the schema and the operations in 2 specific file: schema.json | schema.graphql & operations.json | operations.graphql or at least give better name to the interface & type created in the schema 🤔

@SalahAdDin
Copy link

Do you have any plan to support Subscriptions?

@DigitalCop
Copy link

Do you plan to support behind a firewall/proxy support? Current version does not work due to a CDN request to an external resource. Found a workaround, but still...

@alexandrebodin
Copy link
Member

Linking an issue discussing the Graphql API and some pain points.

@Convly
Copy link
Member Author

Convly commented Nov 24, 2021

Do you have any plan to support Subscriptions?

Little update, a first version of the subscriptions has been implemented in strapi/strapi#11129 and is available in the latest beta

@alexandrebodin
Copy link
Member

alexandrebodin commented Nov 24, 2021

Copy pasting and answering from the issue here :) Issues: strapi/strapi#11649

Current pain points(s)

  1. Defining routes is a pain, especially if compared to what it was like in V3

I'm not sure I understand. What do you mean by routes ? queries & mutations ? resolvers ?

  1. Managing User Permissions is much harder and convoluted (somewhat related to the first pain point)

Can you explain with a given example ?

  1. While extending Mutation the way it currently is needed to is not per se Strapi related problem, it would be amazing if Strapi would handle this by simply creating a new resolver (I believe this is somewhat how it worked in V3)

I'm lost on this one what do you mean ?

  1. The response format is not great for those working with say Apollo Client 3 (or other clients with cache normalization), where the GQL Client relies on a unique identifier inside it GQL type to track the given object. So having the ID inside the attributes would be beneficial. Furthermore the nested data {} inside the response it somewhat verbose and not sure if it's great DX, not only for the sake of the proportionally increasing nullity checks with each nested relation, but also since it - again - increases the complexity for using the GQL Client with cache normalization (this could be solved with something like custom merge functions, but that surely would not be a good DX. Perhaps this could be solved by moving the metadata inside the actual response, say instead of
query {
  posts {
    data {
      id
      attributes {
        title
        ...
      }
    }
    meta {
      pagination {
        ...
      }
    }
  }
}

it could be this:

query {
  posts {
    id
    title
    meta {
      pagination
      ...
    }
  }
}

this format would add the pagination meta for each entry here I don't think this is what you wanted to do ?

@ScottAgirs
Copy link

@alexandrebodin thanks for reposting, very keen on helping in any way I can. Again - sadly, this is a major roadblock for me to switch from V3 to V4, for now, I had to shelf the migration to V4 and keep working with V3.

@krstivojevicv
Copy link

@alexandrebodin Regarding the point 4:

This seems really nice:

query {
  posts {
    id
    title
    meta {
      pagination
      ...
    }
  }
}

If you are not comfortable with meta being inside attributes scope you can use this approach (this is how it works in Directus)

query {
  posts {
    data {
      id
      title
    }
    meta {
      pagination
    }
  }
}

But the first approach seems more convenient and having meta for each entry can actually be an advantage

@ScottAgirs
Copy link

ScottAgirs commented Nov 28, 2021

@alexandrebodin Regarding the point 4:

This seems really nice:

[..]

If you are not comfortable with meta being inside attributes scope you can use this approach (this is how it works in Directus)

query {
  posts {
    data {
      id
      title
    }
    meta {
      pagination
    }
  }
}

[..]

This format would likely still cause headaches because it involves extra layer of nesting and also the actual collection type does not have an identifier on its root level.

However, the first option where meta {} is part of the root collectionType {}
The meta {} could in turn have its own ID attached to it. That setup would likely make working with it on a client a breeze.

@abdonrd
Copy link

abdonrd commented Nov 29, 2021

I'm also more in favor of the flat format.

@valdeniomarinho
Copy link

valdeniomarinho commented Dec 3, 2021

Sadly I still prefer the V3. My opinion is that V4 is way too much verbose and not good DX.

V4 meta{}, data{}, attributes{} : YAGNI and DRY
V3 flat and simple : KISS

@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/discussion-regarding-the-complex-response-structure-for-rest-graphql-developer-experience/13400/1

@tilman
Copy link

tilman commented Dec 23, 2021

Sadly I still prefer the V3. My opinion is that V4 is way too much verbose and not good DX.

meta{}, data{}, attributes{} : YAGNI and DRY flat and simple : KISS

Totally agree to this! Especially if we access deeply nested collections via relations the code get's very long and unreadable.

@abdonrd
Copy link

abdonrd commented Dec 23, 2021

I think that this new structure makes everything much more complex.

With this level of nesting…

I would love to see this structure simplified.

Also this is a plugin, I think a new v5 version could even be created with breaking changes while Strapi stays at v4.

@santiagok986
Copy link

I love V3, simple, easy and quick to use

query($id:ID!){
  recipe(id:$id){
    id
    name
    img
    ingredients
    steps
    category{
      id
      name
      slug
    }
    autor{
      username
    }
  }
}

V4 :( more complex and useless nested

query($id:ID!){
  recipe(id:$id){
    data{
      id
      attributes{
        name
        img
        ingredients
        steps
        category{
          data{
            id
            attributes{
              name
              slug
            }
          }
        }
         autor{
          data{
            attributes{
              username
            }
          }
        }
      }
    }
  }
}

V3 vs V4

Use V3 :) => recipe.name
Use V3 :) => recipe.category.name

Use V4 :/ => recipe.data.attributes.name
Use V4 :( => recipe.data.attributes.category.data.attributes.name

@Sepifanovskiy
Copy link

New response structure results in unreadable code when working with nested collection. But event shallow responses make code ugly with all unnecessary wrappers.

@IsaacDdR
Copy link

IsaacDdR commented Jan 4, 2022

One of the main purposes of GraphQL is lost with this over-engineered "solution"

@Dorifor
Copy link

Dorifor commented Jan 6, 2022

I also think that the new API response is bloated and much harder to read / process in our apps.
It's now harder to make it works and easier to make mistakes (and lose time / sanity)

It's a bit of a shame honestly, hoping to see it take a better direction in the future

@SalahAdDin
Copy link

Is there any way to make out the v4 response version and just use the v3?

Thanks

@nergmada
Copy link

Hi, been using Strapi v3 in a number of projects. The changes in v4 feel like change for the sake of change. Love the new interface etc. but not enough to want to hack together AWS-bucket plugins for media upload to digitalocean etc. (but I digress as this isn't the main comment I want to add).

This change seems arbitrary, unnecessary, and lacking an intuitive feel. I don't tend to read docs before I start firing REST requests or GraphQL at a project, so when I fired my first request at a v4 project, I was already weirded out by the response to a query like below.

query {
  blog {
    title
    body
  }
}

To then find out I had to go two layers deeper to not just data but attributes, this seemed excessive. I personally agree with the sentiment of a flat response structure (as query above shows), but I can see the merit of metadata being kept separate as below

query {
  blog {
    title
    body
    meta {
      published_at
    }
  }
}

but expressing the main content as a sub-object of a sub-object within the response doesn't make any sense because surely the reason we're sending a request is for that data. Admittedly sometimes we only want metadata, but in most cases, I suspect we want something like a title, or a link, or a description from the main content.

I particularly agree with the sentiment about null checks, which I think in both the scenarios above are the same. You technically don't need to do null checks on metadata if content data is there because content data probably wouldn't be sent without metadata (he says brazenly).

@Jacob87
Copy link

Jacob87 commented Jan 29, 2022

rowData?.attributes?.service?.data?.attributes?.price?.data?.attributes?.author?.data?.attributes?.name

😭😭

@pechitook
Copy link

Adding a comment to emphasize something I didn't see anyone else raise: the nesting issue becomes unnecessarily verbose when you're handling media files that are declared as single-file types. Here's a real example from our migration attempt:

V3

query HeaderData {
  siteContent {
    id
    logo {
      url
    }
  }
}

V4

query HeaderData {
  siteContent {
    data {
      id
      attributes {
        logo {
          data {
            attributes {
              url
            }
          }
        }
      }
    }
  }
}

I agree this can come up a bit counter intuitive for graphql folks who are familiar with the flexibility Apollo Client provides. This seems inspired on the Relay pagination strategy, which of course works but what I loved about Strapi is that it was simple by default and you could always extend it as a regular NodeJS server if you needed to.

Keep up the good work!

@stafyniaksacha
Copy link

stafyniaksacha commented Feb 2, 2022

rowData?.attributes?.service?.data?.attributes?.price?.data?.attributes?.author?.data?.attributes?.name

😭😭

This is a pain to handle in typed languages (like in dart, typescript, rust, go).
Is there any new on this ?

edit: found on the forum:

At this time we are not prepared or planning to modify this data structure in the response, the time for that type of modification has passed as it should have been addressed during the RFC process. We do understand that it was not clear at the time during our RFC process this was the case and we are already doing retrospectives to improve this process.

@SalahAdDin
Copy link

Adding a comment to emphasize something I didn't see anyone else raise: the nesting issue becomes unnecessarily verbose when you're handling media files that are declared as single-file types. Here's a real example from our migration attempt:

V3

query HeaderData {
  siteContent {
    id
    logo {
      url
    }
  }
}

V4

query HeaderData {
  siteContent {
    data {
      id
      attributes {
        logo {
          data {
            attributes {
              url
            }
          }
        }
      }
    }
  }
}

I agree this can come up a bit counter intuitive for graphql folks who are familiar with the flexibility Apollo Client provides. This seems inspired on the Relay pagination strategy, which of course works but what I loved about Strapi is that it was simple by default and you could always extend it as a regular NodeJS server if you needed to.

Keep up the good work!

Oh God, my eyes, my EYES!!!

@onurbamaro
Copy link

yeah, I was just setting things up to upgrade my project to V4 until I first came accross the first query with GraphQL.

Dropped the idea of upgrading it until its more like V3.

@cumironin
Copy link

Rest in V4 is very painfull to use, so much easier in V3

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/how-to-use-graphql-with-strapi4/16260/2

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/how-to-use-graphql-with-strapi4/16260/4

@matthill8286
Copy link

I have used V4 for a variety of projects and couldn't understand why everything in the GQL land is ....data.attributes, it's clunky, so I have reverted back to use V3 as the flat structure also follows the graphql spec https://spec.graphql.org/October2021/, i'm also not sure extracting the ID! outside attributes will play well with Apollo caching when you have nested queries, but I guess time will tell.

Whats the reason as so far I can find one, unless i've missed as to why the need to change the GQL offering??

@SalahAdDin
Copy link

Does anyone try to use this plugin?

Does anyone on the Strapi Staff aware of these problems?

@matthill8286
Copy link

matthill8286 commented Apr 2, 2022 via email

@santiagok986
Copy link

The plugin https://market.strapi.io/plugins/strapi-plugin-transformer, only works in REST, how can I make it work in GraphQL ?

@SalahAdDin
Copy link

The plugin https://market.strapi.io/plugins/strapi-plugin-transformer, only works in REST, how can I make it work in GraphQL ?

Really? No way!

@derrickmehaffy
Copy link
Member

@SalahAdDin

Does anyone on the Strapi Staff aware of these problems?

Yes we are aware but functionally we can't change this in v4 as it would be a breaking change and the structure specified is needed for some of our upcoming features (primarily content-versioning)

@SalahAdDin
Copy link

@SalahAdDin

Does anyone on the Strapi Staff aware of these problems?

Yes we are aware but functionally we can't change this in v4 as it would be a breaking change and the structure specified is needed for some of our upcoming features (primarily content-versioning)

Is there any other solution for this? Cause this structure is really horrible, we had to keep the 3 version cause the response format.

@SalahAdDin
Copy link

@SalahAdDin

Does anyone on the Strapi Staff aware of these problems?

Yes we are aware but functionally we can't change this in v4 as it would be a breaking change and the structure specified is needed for some of our upcoming features (primarily content-versioning)

As @alexandrebodin closed this issue, should we understand it will be a solution soon? or won't it be fixed never?

@alexandrebodin
Copy link
Member

Hello @SalahAdDin,

We closed this RFC as this was already implemented and is not the place for issue management.

Please do comment in the forum thread if you want to add any suggestions there. We did share the reasons we need this format in the mid and long term there, this might give you the information you are looking for. If you find some solutions that we could use while addressing our constraints we would be glad to hear about them!

@alexandrebodin alexandrebodin deleted the v4/graphql-api branch July 13, 2022 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.