Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Records #1222

Closed
TimWhiting opened this issue Oct 8, 2022 · 23 comments · Fixed by #1314
Closed

Support for Records #1222

TimWhiting opened this issue Oct 8, 2022 · 23 comments · Fixed by #1314

Comments

@TimWhiting
Copy link

TimWhiting commented Oct 8, 2022

With the upcoming records feature, it would be good to figure out how to serialize those with this package.

I'd propose:

  • Make it an error to mix positional and named fields in an inline type
  • Allow records with only named fields - serializes to a normal JSON map with keys as the field names
  • Allow records with only positional fields - serializes to a normal JSON list (note that the list might not be homogenous)

Allow records with mixed fields with the following qualifications:

  • only via a typedef - so that we don't have to canonicalize structural types.
  • is converted to a JSON map
  • positional fields require a JsonKey annotation with a name

Potential other requirements to consider:

  • Only support JsonKey annotation with typedefs - to make it clear we don't canonicalize structural types

Note that we could probably support not requiring to go through a typedef, it just makes it more error prone for users, because they need to understand that the type is not canonicalized and the annotation would have to be added to all records.

Finally, we would need to consider what annotating a record type by itself (not embedded in an object) with JsonSerializable would look like. I think creating a toJson extension method would be a good approach. Where to put the fromJson method / how to name it is a bit more tricky.

@kevmoo
Copy link
Collaborator

kevmoo commented Oct 10, 2022

I'm not thinking about this too much (yet) – but a good thing to discuss early!

@shilangyu
Copy link

Probably worth reviving now that records reached stable?

@humbertoadan
Copy link

humbertoadan commented May 11, 2023

Probably worth reviving now that records reached stable?

Yes, it's definitely needed now. I'm actually trying JsonKey.toJson and JsonKey.fromJson to use custom conversion on an annotated field which is a list of records. If I do this: List<({String id, int quantity})>? records, it gives me a "Null check operator used on a null value" error.

@kevmoo
Copy link
Collaborator

kevmoo commented May 11, 2023

Hrm...perhaps...

I'm a bit nervous about the effort required here. I could dig a bit.

@kevmoo kevmoo self-assigned this May 11, 2023
@eximius313
Copy link

I've just upgraded to Dart 3.0 and encountered this problem while generating mocks (using Mockito):

[SEVERE] json_serializable on lib/src/my_class.dart (cached):

This builder requires Dart inputs without syntax errors.
However, package:my_pkg/src/my_class.dart (or an existing part) contains the following errors.
my_class.dart:56:10: This requires the 'records' language feature to be enabled.

Try fixing the errors and re-running the build.

I've set

analyzer:
  enable-experiment:
    - records

in analysis_options.yaml but this didn't help

@kevmoo
Copy link
Collaborator

kevmoo commented May 12, 2023

You need to get the latest pkg:analyzer, too!

@eximius313
Copy link

@kevmoo, you mean this one?
I've used the newest one, but error is simmilar:

[SEVERE] mockito:mockBuilder on test/my_test.dart:

Could not format because the source could not be parsed:

line 33, column 22 of .: This requires the 'records' language feature to be enabled.

kevmoo added a commit that referenced this issue May 16, 2023
@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

On the way - #1314

@eximius313
Copy link

awesome!

kevmoo added a commit that referenced this issue May 16, 2023
@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

Published https://pub.dev/packages/json_serializable/versions/6.7.0

@eximius313
Copy link

eximius313 commented May 16, 2023

Umm... I have:

environment:
  sdk: '^3.0.0'

and

  json_annotation: ^4.8.1
  json_serializable: ^6.7.0
  mockito: ^5.4.0
  build_runner: ^2.4.4
  analyzer: ^5.12.0

in pubspec.yaml

and

analyzer:
  enable-experiment:
    - records

in analysis_options.yaml

but I still get the error:

   ╵
line 33, column 894 of .: This requires the 'records' language feature to be enabled.
   ╷
33 │ @override _i4.Future<({Uint8List data, InternetAddress dA})> findIp({required String? name, required int? port, required Duration? timeout, required Function? onTimeout, required Iterable<_i5.InternetAddress>? availableAddresses, required _i6.Uint8List Function(_i5.InternetAddress)? constructMessage, required bool Function(_i6.Uint8List)? isValidResponse, }) => (super.noSuchMethod(Invocation.method(#findIp, [], {#name: name, #port: port, #timeout: timeout, #onTimeout: onTimeout, #availableAddresses: availableAddresses, #constructMessage: constructMessage, #isValidResponse: isValidResponse, }, ), returnValue: _i4.Future<({Uint8List data, InternetAddress dA})>.value(), returnValueForMissingStub: _i4.Future<({Uint8List data, InternetAddress dA})>.value(), ) as _i4.Future<({Uint8List data, InternetAddress dA})>);
   │

@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

You need to change your SDK constraint to ^3.0.0

@eximius313
Copy link

I have it already.
I'm invoking command dart run build_runner build --delete-conflicting-outputs

@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

Have you run pub get/upgrade?

@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

You should NOT need to tweak analysis_options – that won't help the generator!

@eximius313
Copy link

eximius313 commented May 16, 2023

Thank you. pub get/upgrade helped.

I removed:

analyzer:
  enable-experiment:
    - records

from analysis_options.yaml
and:

  analyzer: ^5.12.0

from pubspec.yaml and it finally generated the files.

@eximius313
Copy link

But now the problem is different - in generated file, it generated the imports as:

// Mocks generated by Mockito 5.4.0 from annotations
// in pos_sdk/test/pos_sdk_test.dart.
// Do not manually edit this file.

// ignore_for_file: no_leading_underscores_for_library_prefixes
import 'dart:async' as _i4;
import 'dart:convert' as _i7;
import 'dart:io' as _i5;
import 'dart:typed_data' as _i6;

import 'package:http/http.dart' as _i2;
import 'package:mockito/mockito.dart' as _i1;

but in generated method:

_i4.Future<({Uint8List data, InternetAddress dA})> findIp({

it did not replaced the prefixes, so the file does not compile

@kevmoo
Copy link
Collaborator

kevmoo commented May 16, 2023

This looks like a bug in mockito when dealing w/ Records. 🤷

@Reprevise
Copy link

Reprevise commented May 17, 2023

I was kind of hoping for Records with only positional fields to be serialized as lists so if I have a list in JSON of:

[0.34, 0.123, 0.542, 0.4143]

it would be deserialized into:

(0.34, 0.123, 0.542, 0.4143)

Obviously I understand that whichever way it's implemented people are going to be upset but I think this is far more common than having a map with keys like $1.

@kevmoo
Copy link
Collaborator

kevmoo commented May 17, 2023

@Reprevise – hrm. Yeah.

Or I could do a list if they are all positional.

Hrm. File a separate issue. Happy to entertain for later.

@Reprevise
Copy link

@kevmoo Filed a new issue here #1321

@pintu-elaunch
Copy link

field_rename property is not getting applied for records. It is not renaming record field names. I have set field_rename property in build.yaml, but still it is not getting applied.

build.yaml file:

targets:
  $default:
    builders:
      json_serializable:
        options:
          explicit_to_json: true
          field_rename: snake
          include_if_null: false

@aldee
Copy link

aldee commented Oct 31, 2023

@kevmoo, you mean this one? I've used the newest one, but error is simmilar:

[SEVERE] mockito:mockBuilder on test/my_test.dart:

Could not format because the source could not be parsed:

line 33, column 22 of .: This requires the 'records' language feature to be enabled.

For those who still have this problem, instead of running pub upgrade, which would update a lot of your project's dependencies, you just need to update the package dart_style to the latest version/2.3.2, depending on the version of the package analyzer that you're using

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants