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

Incompatibility with dart 3 #916

Closed
josiahsrc opened this issue May 20, 2023 · 21 comments
Closed

Incompatibility with dart 3 #916

josiahsrc opened this issue May 20, 2023 · 21 comments
Assignees
Labels
bug Something isn't working needs triage

Comments

@josiahsrc
Copy link

Describe the bug
Starting from dart Dart 3.0.1 (shipped with flutter 3.10.1), nested classes produce an InvalidType when generated. E.g. given this code

@freezed
class SinglePage with _$SinglePage {
  factory SinglePage(Page page) = _SinglePage;
}

@freezed
class WidgetType with _$WidgetType {
  const factory WidgetType.page() = Page;
}

Freezed produces this (as one example of the InvalidType occurrence, there are others.

...
mixin _$SinglePage {
  InvalidType get page => throw _privateConstructorUsedError;
  ...
}

To Reproduce

  1. Update your dart version to 3.0.1

  2. Create a dart project with freezed installed

  3. Paste this content into a lib/test.dart file

import 'package:freezed_annotation/freezed_annotation.dart';

part 'test.freezed.dart';

@freezed
class SinglePage with _$SinglePage {
  factory SinglePage(Page page) = _SinglePage;
}

@freezed
class WidgetType with _$WidgetType {
  const factory WidgetType.page() = Page;
}
  1. Generate code

This should generate a file with InvalidType in place of where a nested Page type should be in the freezed file.

Expected behavior
The generated code should use Page instead of InvalidType.

@josiahsrc josiahsrc added bug Something isn't working needs triage labels May 20, 2023
@josiahsrc
Copy link
Author

josiahsrc commented May 20, 2023

I've tried debugging this locally, and it looks like this might be a problem with build-runner itself. When adding this print statement to the generateForData function, the build runner returns InvalidType for any nested freezed types. It looks like this package doesn't have control over this.

@override
  Iterable<Object> generateForData(
    GlobalData globalData,
    Data data,
  ) sync* {
    try {
      final param = data.constructors.first.parameters.allParameters.first;
      print(param.type);
    } catch (e) {
      print(e);
    }
    ...

I'm not sure how best to articulate this to the build runner team outside of the context of the freezed package, since it looks like the invalid types are types that are "yet to be generated". Meaning that, in the example above, the SinglePage tries to generate before the Page code is generated. Any ideas?

@rrousselGit
Copy link
Owner

That's not a new thing in itself.

It's weird that you get InvalidType though. I'll look into that. Normally you'd get dynamic

@rrousselGit
Copy link
Owner

You cannot rely on generates types as input of other generated types in general.

@rrousselGit
Copy link
Owner

Cc @SunlightBro

It may have made sense to throw on invalid type now.

@Carapacik
Copy link

Same problem. I found it after updating analyzer to 5.13.0

@josiahsrc
Copy link
Author

That's not a new thing in itself.

It's weird that you get InvalidType though. I'll look into that. Normally you'd get dynamic

A simple fix could be to find and replace InvalidType with dynamic. This definitely is not robust. It would be better to find the root cause

@josiahsrc
Copy link
Author

I threw together a crude attempt to fix this bug here: #917

I've left this open for edits from maintainers. Feel free to close if this isn't the right approach

@rrousselGit
Copy link
Owner

I'll open an issue on analyzer

Honestly I'd consider this a regression. They made a breaking change without a major release.

@rrousselGit
Copy link
Owner

dart-lang/sdk#52455

@nerder
Copy link

nerder commented May 21, 2023

Adding to this issue, this is something very strange but I get an InvalidType with the first build, but then without changing any relevant code, by just triggering a rebuild it fixes the issue and places the correct type.

You reproduce this here: https://github.com/nerder/invalid-type-repro

I didn't investigate further why this actually happens tho.

@rrousselGit
Copy link
Owner

Do not that relying on generated classes as input of freezed classes was never really supported. I've attempted some workarounds before, but it's really brittle.

So this problem isn't a high priority for me, since this never really worked. It used to generate dynamic, which was already a "bug" in itself.

So I'm fairly tempted to close this as intended and wait for metaprogramming to officially fix this.

@biklas7
Copy link

biklas7 commented May 22, 2023

I'm sorry if I'm saying something dumb as I'm not sure what you mean by used to generate dynamic but in my case the generated files used to infer the correct type, not dynamic, but now I'm getting InvalidType 🤔

I understand this might not be a bug on the freezed package itself, but what do you suggest doing until we have metaprogramming? Lock the analyzer version to 5.12.0? Doing so seems to generate the files correctly.

@rrousselGit
Copy link
Owner

I'd suggest not using generated typed as input of freezed classes ;)

@biklas7
Copy link

biklas7 commented May 22, 2023

I'd suggest not using generated typed as input of freezed classes ;)

Ahhh 💡 I see the problem now! Got it, thanks 😅 That's a bit inconvenient for me though because I'm using GraphQL with code generation and sometimes I need to set those generated types as input of freezed classes which in turn represent my state.

@josiahsrc
Copy link
Author

Do not that relying on generated classes as input of freezed classes was never really supported. I've attempted some workarounds before, but it's really brittle.

So this problem isn't a high priority for me, since this never really worked. It used to generate dynamic, which was already a "bug" in itself.

So I'm fairly tempted to close this as intended and wait for metaprogramming to officially fix this.

It'd be sad to not be able to rely on generated classes as input of freezed. I use that feature quite often. But I see what you're saying and can see how that would be difficult. It sounds like you'd have to derive a dependency graph of generated classes or something similar just to do this

@rrousselGit
Copy link
Owner

There are possible solutions, yes. It would be quite tricky though.

In my humble opinion, analyzer should have InvalidType's getDisplayString output what the user wrote.
The alternative is to effectively completely rewrite Freezed to stop relying on analyzer's DartType. The amount of work is quite large.

The problem is, static metaprogramming ideally will be released at some point. And it doesn't rely on analyzer at all.
So metaprogramming will require a full rewrite of Freezed.

I don't want to rewrite Freezed to fix this temporary issue; only to have to rewrite Freezed again when metaprogramming is released.

@josiahsrc
Copy link
Author

Excellent point. Given that freezed will have to be re-written at that point, perhaps it makes sense to merge #917? It would serve as a bandaid fix for this issue. I've tested the fork on two of my projects with no problems.

@rrousselGit
Copy link
Owner

@josiahsrc If tests are added, sure!

@ValeriusGC
Copy link

Describe the bug Starting from dart Dart 3.0.1 (shipped with flutter 3.10.1), nested classes produce an InvalidType when generated. E.g. given this code

@freezed
class SinglePage with _$SinglePage {

As a workaround one can use somethink like that:

@freezed
class SinglePage with _$SinglePage {
  factory SinglePage(Page page) = _SinglePage;
}

mixin Page {}

@freezed
class WidgetType with _$WidgetType, Page {
  const factory WidgetType.page() = _Page;
}

void test() {
  final Page page = SinglePage(WidgetType.page()).page;
}

Moreover this is works with protopype pattern too:

mixin Page<T> {
  int get id;

  String get title;

  /// For correct overriding in `*.freezed.dart` file
  T get copyWith;
}

@freezed
class WidgetType with _$WidgetType, Page<$WidgetTypeCopyWith<WidgetType>> {
  const factory WidgetType.page({
    @Default(0) int id,
    @Default('') String title,
  }) = _Page;
}

void test() {
  final Page page = SinglePage(WidgetType.page()).page;
  final String title = SinglePage(WidgetType.page()).page.title;
  final Page page2 = SinglePage(WidgetType.page()).page.copyWith(title: 'aaa');
}

@limitless-brain
Copy link

I'm sorry if I'm saying something dumb as I'm not sure what you mean by used to generate dynamic but in my case the generated files used to infer the correct type, not dynamic, but now I'm getting InvalidType 🤔

I understand this might not be a bug on the freezed package itself, but what do you suggest doing until we have metaprogramming? Lock the analyzer version to 5.12.0? Doing so seems to generate the files correctly.

Thank you, Lock the analyzer version to 5.12.0 worked for me.

@rrousselGit
Copy link
Owner

Closing as the latest version should handle this.

A better fix for generated types is in progress, but it'll take a few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants