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

dart:ui types not handled by mobx_codegen properly on Flutter stable/beta #381

Closed
Bwolfs2 opened this issue Jan 8, 2020 · 29 comments · Fixed by #408
Closed

dart:ui types not handled by mobx_codegen properly on Flutter stable/beta #381

Bwolfs2 opened this issue Jan 8, 2020 · 29 comments · Fixed by #408
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Bwolfs2
Copy link

Bwolfs2 commented Jan 8, 2020

Some people in the community are reporting to me that the Color class when placed in Observable is generating Dynamic.

  @observable
  Color colorPrimary = null;

And generated it:

  @override
  dynamic get colorPrimary {
    _$colorPrimaryAtom.context.enforceReadPolicy(_$colorPrimaryAtom);
    _$colorPrimaryAtom.reportObserved();
    return super.colorPrimary;
  }

The Color class is blocked somehow and we are unable to solve it, has anyone had this problem?

@irvine5k
Copy link

irvine5k commented Jan 8, 2020

I have this problem too, should I worry about it with another classes?

@shyndman
Copy link
Collaborator

shyndman commented Jan 8, 2020

This should work, and I have verified that it is working correctly locally. So don't worry. We'll get you fixed up. :)

Can you please verify that you're running the latest version of mobx_codegen (0.4.0+1)? What version does your pubspec.lock say it's using?

@shyndman shyndman added the bug Something isn't working label Jan 8, 2020
@irvine5k
Copy link

irvine5k commented Jan 8, 2020

mobx_codegen: ^0.4.0+1
build_runner: ^1.7.3


 mobx_codegen:
    dependency: "direct dev"
    description:
      name: mobx_codegen
      url: "https://pub.dartlang.org"
    source: hosted
    version: "0.4.0+1"

When generate files throws this error on Class:

'_$Cartouche.backColor' ('List<dynamic> Function()') isn't a valid override of '_Cartouche.backColor' ('List<Color> Function()').dart(invalid_override)

This is the store class:

import 'package:flutter/material.dart';
import 'package:mobx/mobx.dart';

part 'cartouche.g.dart';

class Cartouche = _Cartouche with _$Cartouche;

abstract class _Cartouche with Store {

  @observable
  List<bool> color = List.generate(60, (i) => false);

  @observable
  List<Color> backColor = List.generate(60, (i) => Colors.transparent);

}

Generated code:

@override
  List<dynamic> get backColor {
    _$backColorAtom.context.enforceReadPolicy(_$backColorAtom);
    _$backColorAtom.reportObserved();
    return super.backColor;
  }

  @override
  set backColor(List<dynamic> value) {
    _$backColorAtom.context.conditionallyRunInAction(() {
      super.backColor = value;
      _$backColorAtom.reportChanged();
    }, _$backColorAtom, name: '${_$backColorAtom.name}_set');
  }


@pavanpodila
Copy link
Member

This looks like a bug in the analyzer package. The visitor for field-element itself is returning List<dynamic> instead of List<Color>. @shyndman looks like we may need to see if a more recent version of analyzer fixes this.

  void visitFieldElement(FieldElement element) { } // element is List<dynamic> !!!

Note that this is not a bug with mobx_codgen, but with the analyzer package. We will investigate and get back. Thanks so much for bringing to our attention!

@shyndman
Copy link
Collaborator

shyndman commented Jan 8, 2020

Do you have a local repro @pavanpodila?

@irvine5k, can you attach your entire pubspec.lock?

@pavanpodila
Copy link
Member

@shyndman pls see this commit that shows the test that fails: 2b46073

If you place you debug breakpoint for visitFieldElement, you can see the failing type as List<dynamic>

@irvine5k
Copy link

irvine5k commented Jan 8, 2020

Do you have a local repro @pavanpodila?

@irvine5k, can you attach your entire pubspec.lock?

mobx_color_pubspec.lock.txt

@shyndman
Copy link
Collaborator

shyndman commented Jan 8, 2020

@shyndman pls see this commit that shows the test that fails: 2b46073

If you place you debug breakpoint for visitFieldElement, you can see the failing type as List<dynamic>

@pavanpodila Are you running this test using Flutter's Dart, or a Dart installed standalone?

Including the Flutter dependency isn't enough. build_resolvers, which is responsible for configuring the analyzer, checks to see whether the Dart belongs to Flutter, and if so, includes the dart:ui and Flutter types: https://github.com/dart-lang/build/blob/master/build_resolvers/lib/src/resolver.dart#L326

I was able to uncomment the lines in that commit to see the code generator working appropriately.

@shyndman
Copy link
Collaborator

shyndman commented Jan 8, 2020

Thanks for the lock @irvine5k. Can I get you to try something?

From your project directory, can you run:
flutter clean && flutter packages pub run build_runner build --delete-conflicting-outputs

After that, do you still get the dynamics?

@irvine5k
Copy link

irvine5k commented Jan 8, 2020

Thanks for the lock @irvine5k. Can I get you to try something?

From your project directory, can you run:
flutter clean && flutter packages pub run build_runner build --delete-conflicting-outputs

After that, do you still get the dynamics?

Yes, I do.

@override
  List<dynamic> get backColor {
    _$backColorAtom.context.enforceReadPolicy(_$backColorAtom);
    _$backColorAtom.reportObserved();
    return super.backColor;
  }

@shyndman
Copy link
Collaborator

shyndman commented Jan 9, 2020

Hm. Bear with me.

@irvine5k, I created a very simple project, containing only cartouche.dart and the necessary dependencies. My local version is generating code correctly.

Can you clone https://github.com/shyndman/mobx_list_color_repro, and run flutter packages pub run build_runner build? Does cartouche.g.dart get generated properly there?

@irvine5k
Copy link

irvine5k commented Jan 9, 2020

Hm. Bear with me.

@irvine5k, I created a very simple project, containing only cartouche.dart and the necessary dependencies. My local version is generating code correctly.

Can you clone https://github.com/shyndman/mobx_list_color_repro, and run flutter packages pub run build_runner build? Does cartouche.g.dart get generated properly there?

@shyndman It does not work for me.

@override
  List get backColor {
    _$backColorAtom.context.enforceReadPolicy(_$backColorAtom);
    _$backColorAtom.reportObserved();
    return super.backColor;
  }

  @override
  set backColor(List value) {
    _$backColorAtom.context.conditionallyRunInAction(() {
      super.backColor = value;
      _$backColorAtom.reportChanged();
    }, _$backColorAtom, name: '${_$backColorAtom.name}_set');
  }

@shyndman
Copy link
Collaborator

shyndman commented Jan 9, 2020

hm, interesting that the dynamics are implicit here.

ok, next. what's your flutter --version?

@irvine5k
Copy link

irvine5k commented Jan 9, 2020

@shyndman

Flutter 1.12.13+hotfix.5 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 27321ebbad (4 weeks ago) • 2019-12-10 18:15:01 -0800
Engine • revision 2994f7e1e6
Tools • Dart 2.7.0

@shyndman
Copy link
Collaborator

shyndman commented Jan 9, 2020

k, give me a few

@shyndman
Copy link
Collaborator

shyndman commented Jan 9, 2020

OK, I have a reproduction! Nice.

Give me a few more. :)

@shyndman
Copy link
Collaborator

shyndman commented Jan 9, 2020

Alright, so stable and beta both produce incorrect output.

I have verified that two options fix the output:

  1. Switch to Flutter dev or master channels OR
  2. Downgrade your mobx dependencies to:
    mobx: ^0.3.6
    mobx_codegen: '0.3.11'

@pavanpodila and I will introduce tests and a better process to make sure this doesn't happen again. Sorry about that.

@shyndman shyndman changed the title Problem with Color class Problem with Color (all dart:ui types) on Flutter stable/beta Jan 9, 2020
@shyndman shyndman changed the title Problem with Color (all dart:ui types) on Flutter stable/beta dart:ui types not handled by mobx_codegen properly on Flutter stable/beta Jan 9, 2020
@irvine5k
Copy link

irvine5k commented Jan 9, 2020

No problem, @shyndman.

Thanks for your time.

@mobxjs mobxjs deleted a comment from irvine5k Jan 9, 2020
@mobxjs mobxjs deleted a comment from ricbermo Jan 9, 2020
@mobxjs mobxjs deleted a comment from ricbermo Jan 9, 2020
@shyndman
Copy link
Collaborator

After a bit of research, I believe this to be the issue: dart-lang/build#2528

@shyndman
Copy link
Collaborator

Update: Heard back from the Dart folks, and I'm going to take a stab at fixing this in the build_resolvers library over the next couple days.

@shyndman shyndman self-assigned this Jan 17, 2020
@irvine5k
Copy link

irvine5k commented Jan 22, 2020

@shyndman Any news?

This bug occurred with another classes and was reported in community, unfortunately I can't describe how to reproduce it, seems kinda random.

And it's not the first time that I see like it in the Flutterando community. I asked him(member of community that is struggling with this problem) to contribute in this issue but apparently he's not gonna to do it.

image

Maybe this "Category" is using dart:ui in its members but we need investigate if there's other classes with the same problem.

There's any "workaround" further this below?

Switch to Flutter dev or master channels OR
Downgrade your mobx dependencies to:
mobx: ^0.3.6
mobx_codegen: '0.3.11'

@shyndman
Copy link
Collaborator

Hi @irvine5k,

No progress on build_resolvers yet. I'm going to look soon.

Is this the Category class from the Flutter framework? If so, then yes, it's the same problem.

@irvine5k
Copy link

irvine5k commented Jan 22, 2020

Hi @irvine5k,

No progress on build_resolvers yet. I'm going to look soon.

Is this the Category class from the Flutter framework? If so, then yes, it's the same problem.

According him it's a class generated by Moor

image

Versions:

flutter_mobx 0.3.6
mobx 0.4.0+1
mobx_codegen 0.4.0+1
build_runner 1.7.3

@shyndman
Copy link
Collaborator

shyndman commented Jan 22, 2020

Thanks for the info.

I believe this is a whole other problem, and one that I've seen before — build's inability to analyze the source code of another file generated via build. mobx_codegen can't actually see the Category class, so it resolves to dynamic.

That said, I'm not a build expert. Let me verify that this is indeed the case, and if so, we can start looking into how to fix it.

@irvine5k
Copy link

irvine5k commented Jan 22, 2020

Thanks for the info.

I believe this is a whole other problem, and one that I've seen before — build's inability to analyze the source code of another file generated via build. mobx_codegen can't actually see the Category class, so it resolves to dynamic.

That said, I'm not a build expert. Let me verify that this is indeed the case, and if so, we can start looking into how to fix it.

Thanks for your response @shyndman and your attention for this issue. I'll reporting more bugs and helping with I can.

Let me know about any evolution with this issue, so I can tell to the Flutterando's community.

@shyndman
Copy link
Collaborator

shyndman commented Jan 23, 2020

Sent a pull over to the build project which should resolve dart:ui problems if accepted.
dart-lang/build#2603

@shyndman
Copy link
Collaborator

shyndman commented Jan 24, 2020

Pull merged. We're almost there.

@shyndman
Copy link
Collaborator

Alright, build_resolvers is published as version 1.3.2. I'm putting together a PR right now to set it as our minimum version in mobx_codegen.

@irvine5k
Copy link

Great news o/

shyndman added a commit that referenced this issue Jan 25, 2020
This resolves the `dart:ui` resolution problems we've been seeing with
some versions of Flutter.

Fixes #381
pavanpodila added a commit that referenced this issue Jan 25, 2020
* Upgrade build_resolvers to 1.3.2

This resolves the `dart:ui` resolution problems we've been seeing with
some versions of Flutter.

Fixes #381

* Enclosing dart:ui in backticks

Co-authored-by: Pavan Podila <pavanpodila@users.noreply.github.com>
@pavanpodila pavanpodila added this to the Road to 1.0 milestone Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants