gql-dart / ferry

Stream-based strongly typed GraphQL client for Dart
https://ferrygraphql.com/
MIT License
605 stars 116 forks source link

Regression from 0.10.5: Id is missing in fragments on interfaces when reading the cache #378

Closed ValentinVignal closed 2 years ago

ValentinVignal commented 2 years ago

When reading a query from the cache which contains a fragment on an interface, it crashes because the id is missing from the json and built_value cannot deserialize it.

You can checkout:

https://github.com/ValentinVignal/flutter_app_stable/tree/ferry/bug-cache

or run this code:

// main.dart
import 'package:built_collection/built_collection.dart';
import 'package:ferry/ferry.dart';
import 'package:flutter_app_stable/graphql/__generated__/pokemon_query.data.gql.dart';
import 'package:flutter_app_stable/graphql/__generated__/pokemon_query.req.gql.dart';

void main() {
  final cache = Cache();
  final dataInCache = <GPokemonsData>[];
  cache.writeQuery(GPokemonsReq(), GPokemonsData((data) {
    data.pokemons = ListBuilder();
  }));
  dataInCache.add(cache.readQuery(GPokemonsReq())!);
  print('step 1');
  print(dataInCache);
  cache.writeQuery(GPokemonsReq(), GPokemonsData((data) {
    data.pokemons.add(
      GPokemonsData_pokemons((pokemon) {
        pokemon
          ..id = 'pikachu'
          ..name = 'Pikachu';
      }),
    );
  }));
  dataInCache.add(cache.readQuery(GPokemonsReq())!);  // <- This is where is crashes
  print('step 2');
  print(dataInCache);
}

When I run

flutter pub run lib/main.dart

I get

step 1
[GPokemonsData {
  G__typename=Query,
  pokemons=[],
}]
Unhandled exception:
Deserializing '[__typename, Query, pokemons, [{__typename: Pokemon}]]' to 'GPokemonsData' failed due to: Deserializing '[{__typename: Pokemon}]' to 'BuiltList<GPokemonsData_pokemons>' failed due to: Deserializing '[__typename, Pokemon]' to 'GPokemonsData_pokemons' failed due to: Tried to construct class "GPokemonsData_pokemons" with null field "id". This is forbidden; to allow it, mark "id" with @nullable.
#0      BuiltJsonSerializers._deserialize (package:built_value/src/built_json_serializers.dart:178:11)
#1      BuiltJsonSerializers.deserialize (package:built_value/src/built_json_serializers.dart:124:18)
#2      BuiltJsonSerializers.deserializeWith (package:built_value/src/built_json_serializers.dart:42:12)
#3      GPokemonsData.fromJson (package:flutter_app_stable/graphql/__generated__/pokemon_query.data.gql.dart:30:23)
#4      GPokemonsReq.parseData (package:flutter_app_stable/graphql/__generated__/pokemon_query.req.gql.dart:56:25)
#5      Cache.readQuery (package:ferry_cache/src/cache.dart:141:42)
#6      main (package:flutter_app_stable/main.dart:24:25)
#7      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:19)
#8      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:192:12)
pub finished with exit code 255
GraphQL files ```graphql # schema.graphql interface PokemonInterface { id: ID! name: String! } type Pokemon implements PokemonInterface { id: ID! name: String! } type Query { pokemons: [Pokemon!]! } ``` ```graphql # pokemon_fragment.graphql fragment PokemonFragment on PokemonInterface { id name } ``` ```graphql # pokemon_query.grapql # import './pokemon_fragment.graphql' query Pokemons { pokemons { ... PokemonFragment } } ```
It looks like this is happening on fragments that are on interfaces, it seems to work for fragments on types. This bug was introduced in the version `0.10.5`
knaeckeKami commented 2 years ago

Thanks. I withdrew that version from pub.dev. Unfortunately I don't have access to a laptop today, so I can't give more details at the moment.

If you want to help, I would greatly appreciate it if you could try passing the generated "possibleTypesMap" from the Schema.graphl.dart file to the cache constructor and see if this fixes that.

knaeckeKami commented 2 years ago

Tracked it down to a change in normalize 0.6.0. Possible workaround is to use normalize 0.5.x via dependency overrrides.

knaeckeKami commented 2 years ago

possible fix using ferry_cache from this branch PR: https://github.com/gql-dart/ferry/pull/380 and passing the generated possibleTypesMap from the generated schema.gql.dart file to the Cache constructor.

You also stumbled upon another bug in your repro: If the schema is very simple and contains no inputs, no enums and no unions, no schema.gql.dart file is created. This should almost never happen in production scenarios though so it's probably no big deal.

You can try it out in your repro by adding some dummy input like

input PokemonInput {
  name: String!
}

to the schema.graphql file in your repro-project.

I assume you ran into this on a real-world project, I would be grateful if you could try the fix on this project.

TL;DR:

use ferry_cache from

 ferry_cache:
   git:
       url: https://github.com/gql-dart/ferry.git
       ref: fix/cache_possible_types_fix
       path: packages/ferry_cache

and change the instantiation of your cache object to Cache(possibleTypes: possibleTypesMap, ....) (with possibleTypesMap imported from your schema.gql.dart file) and see if the problem persists.

ValentinVignal commented 2 years ago

I confirm that using

 ferry_cache:
   git:
       url: https://github.com/gql-dart/ferry.git
       ref: fix/cache_possible_types_fix
       path: packages/ferry_cache

and adding a

input PokemonInput {
  name: String!
}

on the repo I provided fixes the issue, I will need some time to try it out on my real-world project.

ValentinVignal commented 2 years ago

@knaeckeKami The fix is working on our real-world app too ! 🎉

knaeckeKami commented 2 years ago

Great. Released ferry 0.11.0 (since the possibleTypesMap requirement for full cache compatibility is a breaking change)

ValentinVignal commented 2 years ago

As an update: it works on the minimal repo I provided in the issue 🎉 I still need to try it out on our real-app world

ValentinVignal commented 2 years ago

It is working on our real-app world ! 🎉 Thank you a lot for the support