aerogear / graphback

Graphback - Out of the box GraphQL server and client
https://graphback.dev
Apache License 2.0
409 stars 73 forks source link

Generated Code #1501

Open kingsleyzissou opened 4 years ago

kingsleyzissou commented 4 years ago

Due to an issue with Apollo client, using different fragments in our types causes some unexpected issues with the cache updates.

In our generated code in the Datasync-Starter app, our find all query is using an expanded fragment and our new task subscription is just using a regular fragment, as below:

findTasks query

export const findTasks = gql`
  query findTasks($filter: TaskFilter, $page: PageRequest, $orderBy: OrderByInput) {
    findTasks(filter: $filter, page: $page, orderBy: $orderBy) {
      items {
        ...TaskExpandedFields
      }
      offset
      limit
    }
  }

  ${TaskExpandedFragment}
`

newTask subscription

export const newTask = gql`
  subscription newTask($filter: TaskSubscriptionFilter) {
  newTask(filter: $filter) {
      ...TaskFields
  }
} 

  ${TaskFragment}
`

As a current workaround, I have updated the generated code so the subscription and query both use the TaskExpandedFragment, but this is obviously not ideal.

kingsleyzissou commented 4 years ago

@wtrocki @craicoverflow @machi1990

wtrocki commented 4 years ago

CC @kingsleyzissou I put some thoughts into this and it is not actually such easy fix as aligning to the same fragment as subscriptions in our cases almost always use root.

Then we also have outstanding work on using variables to enable fragment creation.

1370

In the context of the discussion we had on https://github.com/aerogear/offix/issues/505 I think we could:

1) Pull out client generation from the Graphback (still using graphback core though) to work with the datastore

As we offer more robust CRUD capabilities we cannot longer cather for some various use cases in the apps. We need client-side generation queries for datastore which will be ignoring relationships in queries.

2) Think about offix based generator that will be supporting DataStore with Types and supporting graphql queries for the CRUD model.

3) Ignore completely userspace - queries will need to be written by hand but they can use generated fragments (not 100% sure about this as it is valuable to have something like this but when having multiple relationships we will be forced to do some extra vars that will make things more complex that they should:

For more can we get this fixed now we have 3 options:

1) Hack things on the Apollo client side 2) Remove expanding relationships in data sync starter (we have flag to configure level of expansion) 3) Make a dirty change in graphback to return the same (TaskFields) fragment.

Opinions?

CC @Eunovo

Eunovo commented 4 years ago

Queries don't need to be generated for DataStore. DataStore can create sync queries and subscriptions at runtime.

I don't see what graphback-client has to do with it. I think it's better we generate the proposed schema config with separate offix based plugin

craicoverflow commented 4 years ago

Is this still an issue?

wtrocki commented 4 years ago

@craicoverflow DataSync starter were not updated for quite long time. We should do that for the next release. This is still issue