fromJson with DateTime (and custom types) expects String

[codakkk]

I have this schema:

input TripsSetInput {
  arrivalDate: date
  arrivalHour: timetz
  ....
}

where date and timetz are scalars defined like this in the build.yaml:

targets:
  $default:
    builders:
      graphql_codegen:
        options:
          ....
          scalars:
            date:
              type: DateTime
            timetz:
              type: TimeTz
              fromJsonFunctionName: timeTzFromJson
              toJsonFunctionName: timeTzToJson
              import: "package:utils/src/types/time_tz.dart"
            .....

Now the input model fromJson method generated is the following:

factory InputTripsSetInput.fromJson(Map<String, dynamic> data) {
    final result$data = <String, dynamic>{};
    .....
    if (data.containsKey('arrivalDate')) {
      final l$arrivalDate = data['arrivalDate'];
      result$data['arrivalDate'] = l$arrivalDate == null
          ? null
          : DateTime.parse((l$arrivalDate as String));
    }
    if (data.containsKey('arrivalHour')) {
      final l$arrivalHour = data['arrivalHour'];
      result$data['arrivalHour'] =
          l$arrivalHour == null ? null : timeTzFromJson(l$arrivalHour);
    }

The problem here is that it's trying to use DateTime.parse instead of using l$arrivalDate as DateTime. Is this an error or an expected output? Doing so, we have two limitations:

• The generator can only use DateTime.parse preventing us from using a different parser
• Passing a DateTime instance without converting it manually to string thus doing a DateTime -> String -> DateTime

I was thinking about two workarounds:

• Check for runtime type and act based on it. If it's a String run DateTime.parse otherwise if runtime type is DateTime assign it.
• Just pass DateTime instance.

My proposal is to just use the first "workaround" which would allow the method to handle both String and DateTime types gracefully, thus avoiding unnecessary parsing and potential errors or performance issues with unnecessary string-to-date conversions. And the same could be implemented for other types like for arrivalHour in my example below.

factory InputTripsSetInput.fromJson(Map<String, dynamic> data) {
    final result$data = <String, dynamic>{};
    .....
    if (data.containsKey('arrivalDate')) {
      final l$arrivalDate = data['arrivalDate'];
      result$data['arrivalDate'] = l$arrivalDate == null
          ? null
          : l$arrivalDate is DateTime ? l$arrivalDate : DateTime.parse((l$arrivalDate as String));
    }
    if (data.containsKey('arrivalHour')) {
      final l$arrivalHour = data['arrivalHour'];
      result$data['arrivalHour'] =
          l$arrivalHour == null ? null : l$arrivalHour is TimeTz ? l$arrivalHour : timeTzFromJson(l$arrivalHour);
    }
    .....

[github-actions]

👋 @codakkk
Thank you for raising an issue. I will investigate the issue and get back to you as soon as possible.
Please make sure you have provided enough context.

This library is created and maintained by me, @budde377. Please consider supporting my work and ensure our survival by donating here.

[budde377]

So, if I understand you correctly, you want custom parsing of DateTime. If this is the case, you can follow the pattern you are using for timetz and provide the fromJsonFunctionName and toJsonFunctionName. See #349 for an example.

[codakkk]

So, if I understand you correctly, you want custom parsing of DateTime. If this is the case, you can follow the pattern you are using for timetz and provide the fromJsonFunctionName and toJsonFunctionName. See #349 for an example.

Not correctly. The idea is to check runtime type before doing any kind of parse (in the example DateTime.parse).

[budde377]

But `DateTime` is not a valid json type, so this check does not make sense to me. Are you able to explain why providing your parsing logic through the function as mentioned above does not work in your example?

…

On Sun, 14 Apr 2024 at 21:01, Ciro Carandente (Coda) < ***@***.***> wrote: So, if I understand you correctly, you want custom parsing of DateTime. If this is the case, you can follow the pattern you are using for timetz and provide the fromJsonFunctionName and toJsonFunctionName. See #349 <#349> for an example. Not correctly. The idea is to check runtime type before doing any kind of parse (in the example DateTime.parse). — Reply to this email directly, view it on GitHub <#348 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2UFS2OG34VPMKP3IRQVCLY5LOCHAVCNFSM6AAAAABGGJHM7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJUGE3DSNRXHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[codakkk]

But DateTime is not a valid json type, so this check does not make sense to me. Are you able to explain why providing your parsing logic through the function as mentioned above does not work in your example?
…
On Sun, 14 Apr 2024 at 21:01, Ciro Carandente (Coda) < @.> wrote: So, if I understand you correctly, you want custom parsing of DateTime. If this is the case, you can follow the pattern you are using for timetz and provide the fromJsonFunctionName and toJsonFunctionName. See #349 <#349> for an example. Not correctly. The idea is to check runtime type before doing any kind of parse (in the example DateTime.parse). — Reply to this email directly, view it on GitHub <#348 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2UFS2OG34VPMKP3IRQVCLY5LOCHAVCNFSM6AAAAABGGJHM7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJUGE3DSNRXHA . You are receiving this because you were mentioned.Message ID: @.>

Well, why should we limitate fromJson only to actual strings? Nothing should prevents me from creating my input object like this:

final input = InputTripsSetInput.fromJson({
'arrivalDate': DateTime.now();
});

Unluckly my specific case requires me to use fromJson to create InputTripsSetInput and other types but without knowning the real member type (they are all dynamics).

[codakkk]

Just to give you some background about my actual code, I have a library that tells me each time a cell changes. This is the code:

Future<void> _onChanged(PlutoGridOnChangedEvent event) async {
    final data = <String, dynamic>{};

    final id = event.row.cells['id']!.value;
    if (event.value is Map) {
      final m = event.value as Map;
      for (final key in m.keys) {
        data[key] = m[key];
      }
    } else {
      data[event.column.field] = event.value;
    }

    final request = await context.client.mutateUpdateTrip(
      OptionsMutationUpdateTrip(
        variables: VariablesMutationUpdateTrip(
          id: id,
          $set: InputTripsSetInput.fromJson(data),
        ),
      ),
    );

    if (request.hasException && mounted) {
      context.showErrorAlert(
        'pages.tripList.errors.genericError'.tr(
          namedArgs: {
            'fieldName': event.column.field,
          },
        ),
      );
    }
  }

Doing so I can just create InputTripsSetInput based on changed cell (because the cell key is the same as the graphql field name)

[budde377]

The fromJSON method assumes that the input is the result of json serialisation, i.e. records, lists and primitives. To change this I think we need a convincing argument. The fact that you can not be bothered to create a custom scalar parsing using existing functionality is not enough. Can you please confirm or deny that providing your own parsing logic for the DateTime solves your use case?

[codakkk]

The fromJSON method assumes that the input is the result of json serialisation, i.e. records, lists and primitives. To change this I think we need a convincing argument. The fact that you can not be bothered to create a custom scalar parsing using existing functionality is not enough. Can you please confirm or deny that providing your own parsing logic for the DateTime solves your use case?

Well, it’s not about being 'bothered,' but I understand your point and I disagree. I used the ferry library, and the code provided worked fine, accommodating different use cases like mine without the need for extra code (it’s literally just a type check, lol).

Anyway, I've already implemented the custom scalar parsing, but I thought it could be a positive change for the library, that’s all.