[gql_build] Add immutability features

[smkhalsa]

I just wanted to start a discussion about the possibility of building data as built_values rather than storing the Json object and using getters.

I find the immutability and serializability of built_value to be compelling, although it would add some complexity.

Thoughts?

[klavs]

The data class is just a viewer on top of a serialized JSON, so you can "serialize" it by calling .data. They are not quite immutable, but I think we can achieve that by improving the data_builder without using built_value.

To me it feels like built_value is solving a slightly different problem than the problem we have.

That said, I'm happy to hear more about concrete examples where our data getters fall short.

One known issue is that field values get instantiated on every read. But we can get around it by caching values on first read.

[smkhalsa]

another option:

https://github.com/rrousselGit/freezed

[micimize]

I'm using built_value in major_graphql - freezed has an impressive api, didn't know about it before / it is young.

built_value has it's own serialization solution which I ended up using, but I think json serializable might have been better-suited. It's better documented, and the built value solution prioritizes his custom format.

Other than that, syntax and maturity are the major differences. freezed looks to have a relatively smaller surface area, so maturity may be less of an issue. I'm intending to lean in to built_value's closure-based api with things like flutter integration etc – we'll see how it pans out though

[klavs]

@micimize It was some time ago, but I felt that json serializable is incompatible with what we need here. Has it worked well for you?

If I remember correctly, it was about the possibility of representing optional field arguments variables.

[smkhalsa]

I'd like to do some work on adding immutability features to the builders, starting with data_builder.

From my perspective, the following are important features that are currently missing from generated data classes:

1. Manual Instantiation: We currently only generate getters within data_builder. Some use cases for this include creating optimistic responses or mocking data for unit tests.
2. Immutability with == / hashCode equality.
3. Deep copy / updates: One use case example would be reading data from a cache, updating a field, and writing it back to the cache.

@micimize @klavs is there any other functionality that you'd want to see in generated data classes?

built_value and freezed both seem like reasonable choices for the above features. I'm leaning towards freezed since it seems leaner.

One known issue is that field values get instantiated on every read. But we can get around it by caching values on first read.

Freezed's late functionality seems like it'd solve this issue.

[klavs]

@smkhalsa, at this point I do not have any codegen project to guide me so I'm happy to give you the ownership of the codegen packages.

I've been doing some GraphQL codegen in the JS-land recently and I've found that using simple String templates are easier to use than the builder pattern.

Using built_value or freezed would mean that the build process becomes two-stage, which is fine if it works well.

[micimize]

I ran into no real issues using json_serializable that I can recall.

Idk how much "leaner" freezed really is – it adds some really magic-seeming helpers.
But, the sealed union support kind of make it look like a better fit for modeling graphql. Both are kinda in in their own worlds, but code using freezed will probably be easier to migrate to native equivalents if/when they are developed.

Serialization and thus custom scalar support is important to me – not sure where that currently stands.

If the models are meant to be instantiable then$ListPokemon$pokemons should be something more indicative of that imo. I wish we could do ListPokemon.Pokemon

I eventually want to get to a fully typed cache that is essentially a partial of the remote schema, which means

• schema type (partials) availability on the client
• properly modeling the results of fields with arguments

[micimize]

I also use string templating in major_graphql_generator – you can chain builders which I think works alright.

one major tick in built_value's favor is that major_graphql_generator could be used as a starting point.

[smkhalsa]

Freezed doesn't support implementing interfaces from other freezed classes, which will be an issue since we rely on interfaces to implement fragments / unions.

rrousselGit/freezed#156

[rrousselGit]

Freezed doesn't support implementing interfaces from other freezed classes, which will be an issue since we rely on interfaces to implement fragments / unions.

If that's needed, Freezed could generate the methods as extensions instead of methods.

This should allow things like:

@freezed
abstract class Tiny with _$Tiny {
  factory Tiny({ String name }) = _Tiny;
}

@freezed
abstract class Small with _$Small implements Tiny {
  factory Small({ String name, int age }) = _Small;
}

[smkhalsa]

@rrousselGit Thanks for chiming in here.

If that's needed, Freezed could generate the methods as extensions instead of methods.

I've started working on incorporating built_value, but if you decide to make this change to freezed, I'll definitely look at switching over.

[smkhalsa]

@rrousselGit I'm running into some issues using built_value, and I'd definitely love to try freezed instead if you're able to support the above use case.

[rrousselGit]

I'll give this a look tomorrow, I have to think this through just in case.

Would you mind describing why you need that in the first place?
To make sure this is truly the proper solution.

[smkhalsa]

Sure. Here's some example code that gql_code_builder might produce currently:

class $AllSongsQuery {
  const $AllSongsQuery(this.data);

  final Map<String, dynamic> data;

  List<$AllSongsQuery$Song> get Song => data['Song'] == null
      ? null
      : (data['Song'] as List)
          .map((dynamic e) => $AllSongsQuery$Song((e as Map<String, dynamic>)))
          .toList();
}

class $AllSongsQuery$Song implements $SongFragment {
  const $AllSongsQuery$Song(this.data);

  final Map<String, dynamic> data;

  String get id => (data['id'] as String);
  String get name => (data['name'] as String);
  String get fileUrl => (data['fileUrl'] as String);
  String get imageUrl => (data['imageUrl'] as String);
  String get albumName => (data['albumName'] as String);
  String get artistName => (data['artistName'] as String);
}

class $AudioFragment {
  const $AudioFragment(this.data);

  final Map<String, dynamic> data;

  String get id => (data['id'] as String);
  String get name => (data['name'] as String);
  String get fileUrl => (data['fileUrl'] as String);
}

class $SongFragment implements $AudioFragment {
  const $SongFragment(this.data);

  final Map<String, dynamic> data;

  String get id => (data['id'] as String);
  String get name => (data['name'] as String);
  String get fileUrl => (data['fileUrl'] as String);
  String get imageUrl => (data['imageUrl'] as String);
  String get albumName => (data['albumName'] as String);
  String get artistName => (data['artistName'] as String);
}

[smkhalsa]

We may be able to get away with only building $AllSongsQuery and $AllSongsQuery$Song as freezed classes, and have $AudioFragment and $SongFragment be plain abstract classes, thereby bypassing the current limitation.

However, there may be edge cases where this doesn't work (e.g. if the user for some reason needs to be able to instantiate a GraphQL fragment class).

[rrousselGit]

I don't quite understand. What would that code looks like with freezed, and why is it generated this way?

[smkhalsa]

I don't quite understand. What would that code looks like with freezed, and why is it generated this way?

With freezed, it would look something like:

@freezed
abstract class AllSongsQuery with _$AllSongsQuery {
  factory AllSongsQuery({
    List<AllSongsQuery$Song> Song,
  }) = _$AllSongsQuery;
}

@freezed
abstract class AllSongsQuery$Song
    with _$AllSongsQuery$Song
    implements SongFragment {
  factory AllSongsQuery$Song({
    String id,
    String name,
    String fileUrl,
    String imageUrl,
    String albumName,
    String artistName,
  }) = _$AllSongsQuery$Song;
}

@freezed
abstract class AudioFragment with _$AudioFragment {
  factory AudioFragment({
    String id,
    String name,
    String fileUrl,
  }) = _$AudioFragment;
}

@freezed
abstract class SongFragment with _$SongFragment implements AudioFragment {
  factory SongFragment({
    String id,
    String name,
    String fileUrl,
    String imageUrl,
    String albumName,
    String artistName,
  }) = _$SongFragment;
}

This allows common usage of data fetched using different GraphQL queries. For example, we might also have a SongDetailsQuery, and both AllSongsQuery$Song and SongDetailsQuery$Song could be passed into a common flutter widget that accepts a SongFragment.

[smkhalsa]

However, as I mentioned above, we may not need AudioFragment and SongFragment to be freezed classes, in which case freezed should probably work fine as it is.

@klavs @micimize Do you see any reason why generated fragment classes should be instantiable?

[smkhalsa]

@rrousselGit rrousselGit/freezed#70 looks like it will also be an issue for us. Since we are generating freezed classes based on the user-defined graphql models, we cannot guarantee that there won't be collisions.

[rrousselGit]

I don't mind solving it, but I am not aware of any simple solution.

This is something that all code-generators suffers from.
How is your package dealing with it? 😊

[smkhalsa]

gql_build uses code_builder which has an Allocator class that handles import aliasing.

[micimize]

I think Allocator is for referencing imports in generated code, whereas the freezed issue is about referencing user imported libraries with aliases.

It looks like built_value parses the library's ast. I revived an issue on source_gen asking for a generic solution/recipe dart-lang/source_gen#462

[rrousselGit]

Yeah, but that falls in the "not so simple category" It's quite tedious, as it's not solved in a single location and needs to be done every time we want to use a type. I can easily see me forgetting one type in a specific part of the generated code. So I need to think this through Le jeu. 14 mai 2020 à 15:52, Michael Joseph Rosenthal < [email protected]> a écrit :

…

I think Allocator is for referencing imports in generated code, whereas the freezed issue is about referencing user imported libraries with aliases. It looks like built_value parses the library's ast <https://github.com/google/built_value.dart/pull/703/files#diff-e86ce28f4538fe998a497569897b4a33>. I revived an issue on source_gen asking for a generic solution/recipe dart-lang/source_gen#462 <dart-lang/source_gen#462> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#35 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEZ3I3IAW3VHBLDSLD3RHPTRRQATFANCNFSM4KIWM7FQ> .

[smkhalsa]

closed in #110