2.4.3 - Breaking Changes + Issues

[AlexDochioiu]

Breaking Change (1):

For private classes, the name of the FromJson changed

Take this class for example:

@freezed
class _SwapAssetDto with _$_SwapAssetDto {
  const _SwapAssetDto._();

  const factory _SwapAssetDto({
    @JsonKey(name: "unit") required String unit,
    @JsonKey(name: "name") required String name,
    // ignore: unused_element
    @JsonKey(name: "decimals") int? decimals,
    // ignore: unused_element
    @JsonKey(name: "ticker") String? ticker,
  }) = __SwapAssetDto;

  // 2.4.3
  factory _SwapAssetDto.fromJson(Map<String, dynamic> json) => _$SwapAssetDtoFromJson(json);

  // 2.4.2 and below
//factory _SwapAssetDto.fromJson(Map<String, dynamic> json) => _$_SwapAssetDtoFromJson(json);
}

The underscore after the $ is now missing.
`

Breaking Change (2):

Cannot generate toJson anymore. This used to work with 2.4.2.

P.s. If you run the code below with 2.4.2, you'll need to fix the fromJson methods (mentioned in first issue)

// ignore_for_file: invalid_annotation_target

import 'package:freezed_annotation/freezed_annotation.dart';

part 'swap_assets.freezed.dart';

part 'swap_assets.g.dart';

@freezed
class _SwapAssetDto with _$_SwapAssetDto {
  const _SwapAssetDto._();

  const factory _SwapAssetDto({
    @JsonKey(name: "unit") required String unit,
    @JsonKey(name: "name") required String name,
    // ignore: unused_element
    @JsonKey(name: "decimals") int? decimals,
    // ignore: unused_element
    @JsonKey(name: "ticker") String? ticker,
  }) = __SwapAssetDto;

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

@freezed
class _SwapAssetsDto with _$_SwapAssetsDto {
  const _SwapAssetsDto._();

  const factory _SwapAssetsDto({
    @JsonKey(name: "response_time_utc") required DateTime responseTime,
    @JsonKey(name: 'verified') required List<_SwapAssetDto> verifiedAssets,
    @JsonKey(name: 'unverified') required List<_SwapAssetDto> unverifiedAssets,
  }) = __SwapAssetsDto;

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

@Freezed(fromJson: false, toJson: false)
class SwapAssets with _$SwapAssets {
  const SwapAssets._();

  const factory SwapAssets({
    required DateTime responseTime,
    required List<SwapAsset> assets,
  }) = _SwapAssets;

  factory SwapAssets.fromJson(Map<String, dynamic> json) {
    return SwapAssets._fromDto(_SwapAssetsDto.fromJson(json));
  }

  factory SwapAssets._fromDto(_SwapAssetsDto dto) {
    return SwapAssets(responseTime: DateTime.now(), assets: []);
  }

}

@freezed
class SwapAsset with _$SwapAsset {
  const SwapAsset._();

  const factory SwapAsset({
    required String dotSeparatedUnit,
    required String unit,
    required bool verified,
    required bool isAda,
    required int? decimals,
    required String policy,
    required String hexAssetName,
    required String displayTitle,
    required String? displaySubtitle,
    required String imageModel,
  }) = _SwapAsset;

  factory SwapAsset.fromJson(Map<String, dynamic> json) => SwapAsset._fromDto(
        _$SwapAssetDtoFromJson(json),
        false,
      );

  factory SwapAsset._fromDto(_SwapAssetDto dto, bool verified) => SwapAsset._fromData(
        unit: dto.unit,
        name: dto.name,
        ticker: dto.ticker,
        decimals: dto.decimals,
        verified: verified,
      );
}

P.s. this is a slightly shorter/simplified version of my actual code.

[rrousselGit]

The json rename sounds more like a bug fix than a breaking change to me.
The generated name was incorrect.

I'll investigate the other issue though

[AlexDochioiu]

Hmm. Even if you were to argue that it's a bug fix and not an introduced bug. It's still a breaking change since it's breaking the pre-existing working code.

P.s. I don't mind updating it on my end if you decide it should stay like this starting with 2.4.3. I do expect however you'll receive more issues reported on it.

[alguintu]

Updating to 2.4.3 broke our existing Freezed with Hive.

@freezed
class Cart extends HiveObject with _$Cart {
  Cart._();

  @HiveType(typeId: HiveTypes.cart)
  factory Cart({
    @HiveField(0) required int id,
    @HiveField(1) required Store store,
    @HiveField(2) List<CartItem>? cartItems,
  }) = _Cart;

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

Generated Hive TypeAdapters in .g.dart changed from

class CartAdapter extends TypeAdapter<_$_Cart> {}

to

class CartImplAdapter extends TypeAdapter<_$CartImpl> {}

Importing the new Adapters fixed the issue, but quite tedious.

[rrousselGit]

_$CartImpl isn't meant to be used. Its name shouldn't matter for you. So it's a breach of contract if renaming that class broke your code. Sounds like you used something which you shouldn't have (or hive did in this case).

[rrousselGit]

@AlexDochioiu I checked your code but it works for me.
The issue is, instead of with _$_SwapAssetDto it should be _$SwapAssetDto. So there's no bug involved.

I'll leave this open to discuss the possibly breaking nature of this change – even if it's meant to be bugfixes.

If you think this should be a 3.0.0 version, please 👍 the issue

[AlexDochioiu]

@rrousselGit For me it would make sense to be a 3.0.0 as long as you can remove 2.4.3 from pub.dev (so that people don't get auto-updated to this version using a simple upgrade command).

If you plan to keep 2.4.3 (or if it cannot be removed from pub.dev), then I guess there's no real benefit to upgrade the major version anymore.

[rrousselGit]

I have a few days left to pull down the version.
Honestly I'm still not convinced that it is a problem.

Hive's case is problematic, but that's because the use-case didn't respect privacy rules (using stuff that is internal to Freezed).

Whereas while the $ -> _$ thing is bothering, it was a bug. And bugfixes shouldn't count as breaking changes.

[AlexDochioiu]

Was it really a bug though? I'd argue the old behaviour was the better/correct one.

While dart uses the leading underscore to define visibility, it's also still part of the class/property/etc name. And the language doesn't mind if you define two different classes/variables/etc as whatever and _whatever. They are seen as completely independent.

The following scenario (for example) works with 2.4.2 but wouldn't work with 2.4.3 since the from json method would have the same name.

import 'package:freezed_annotation/freezed_annotation.dart';

part 'test.freezed.dart';

part 'test.g.dart';

@freezed
class Test with _$Test {
  const Test._();
  const factory Test(String entry) = _Test1;
  
  factory Test.fromJson(Map<String, dynamic> json) => _$TestFromJson(json);
}

@freezed
class _Test with _$_Test {
  const _Test._();
  const factory _Test(String entry) = _Test2;

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

As for the other part. I think a breaking change should only be defined based on whether it starts breaking the build phase. I think the cause of the change (improvement/bug fixing/etc) is irrelevant for the definition.

[AlexDochioiu]

P.s. I hope I don't come too harsh. In all honesty I personally think freezed is in top 3 best and most important libraries for flutter/dart (especially as someone migrating over from kotlin). So really appreciate your work with it.

[rrousselGit]

It is a bug. There have been numerous bug reports about this. Writing $ does not respect Dart naming conventions and triggers warnings
Various packages using Freezed have their pub score hit because of this for example.

If you look at other code generators, they correctly change _foo into _$foo instead of _$_foo.

And generally speaking, bug fixes don't count as breaking changes.

[sososdk]

The change of 2.4.3 does more harm than good.

When foo and _foo class exist at the same time, the compilation will report an error.

[wxxedu]

A relative minor issue with this change is that the snippets break after this change.

[momrak]

I bumped into this issue as well now and I think doing this as a major version change would have been a better experience as a user of the package.

The main reason is that when I bump packages and only touch the minor and patch versions I expect things to continue working.
Even though I guess this does not break any API per se, it requires me as a user to make changes to my code and then I would prefer having it as a major change. Then I would expect having to go to the changelog or some migration guide to see what I need to do in order for my code to continue working. With this approach I spent an unnecessary amount of time finding this issue and finding the fix.

Having a section in the docs about using Freezed with private classes would probably have saved me some time here as well. The docs was the first place I stared looking when getting the error.