rrousselGit / freezed

Code generation for immutable classes that has a simple syntax/API without compromising on the features.
https://pub.dev/packages/freezed
1.93k stars 237 forks source link

Type with typedef get changed by freezed #753

Open saddig opened 2 years ago

saddig commented 2 years ago

Describe the bug freezed's code generation changes the type of fields defined with a typedef type:

In Isar, Id is defined as

/// Type to specify the id property of a collection.
typedef Id = int;

Related issue opened in Isar: https://github.com/isar/isar/issues/639

To Reproduce

import 'package:freezed_annotation/freezed_annotation.dart';
import 'package:isar/isar.dart';

part 'order.freezed.dart';
part 'order.g.dart';

@Freezed(makeCollectionsUnmodifiable: false)
@Collection(ignore: {'copyWith'})
class Order with _$Order {
  factory Order({
    Id? localId,
    required String id,
     }) = _Order;

  factory Order.fromJson(Map<String, dynamic> json) => _$OrderFromJson(json);
}

order.freezed.dart:

/// @nodoc
mixin _$Order {
  int? get localId => throw _privateConstructorUsedError;

Expected behavior I'd expect freezed to pass the type Id to the generated class instead of passing int.

Using: freezed: ^2.1.0+1 freezed_annotation: ^2.1.0

isar: 3.0.0-dev.14 isar_flutter_libs: 3.0.0-dev.14 isar_generator: 3.0.0-dev.14

rrousselGit commented 2 years ago

It's not supposed to matter.

The true issue I would say is what isar does with the typdef. It's using typedefs in a way that breaks the equality, where Id != int

I believe it'd be more logical to do:

@Id()
int id

instead of:

Id id
simc commented 2 years ago

The reason why it is done this way is that

@collection
class Model {
  Id id;

  float floatProp;

  List<byte> byteList;

  List<short> shortList;
}

is much more readable than the previous

@collection
class Model {
  @id
  int id;

  @size32
  double floatProp;

  @size8
  List<int> byteList;

  @size32
  List<int> shortList;
}

and these typedefs cannot be used incorrectly. You can never try to create a double id or @size32 String.

Furthermore web and native targets have different Id types. Id is num for web and int for native targets.

rrousselGit commented 2 years ago

I understand the desire, but it's not really a typedef anymore though.

I'd be happy to review a PR for this, but I don't plan on working on this.

stevehayles commented 1 year ago

For anyone struggling with this, see workaround here

ookami-kb commented 1 year ago

It's not supposed to matter.

It actually does matter. Consider this example:

/// a.dart

class A {}

/// b.dart

import 'a.dart';

typedef B = A;

/// c.dart

import 'package:freezed_annotation/freezed_annotation.dart';

import 'b.dart';

part 'c.freezed.dart';

@freezed
class C with _$C {
  const factory C(B b) = _C;
}

void main() {
  final c = C(B());
  print(c.b);
}

In this case, c.b type is not properly resolved because freezed generates _C as:

mixin _$C {
  A get b => throw _privateConstructorUsedError;

  @JsonKey(ignore: true)
  $CCopyWith<C> get copyWith => throw _privateConstructorUsedError;
}

But A type is not imported (only B is).