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

deliveryChanges does not work with Dart2 (with solution) #54

Closed
jodinathan opened this issue Feb 22, 2018 · 12 comments
Closed

deliveryChanges does not work with Dart2 (with solution) #54

jodinathan opened this issue Feb 22, 2018 · 12 comments

Comments

@jodinathan
Copy link

jodinathan commented Feb 22, 2018

Extend ChangeNotifier with your own ChangeRecord, like:

class MyRecord extends ChangeRecord {
}
class MyClass extends ChangeNotifier<MyRecord> {
}

once you add some notifications, dart throws an error on:

bool deliverChanges() {
    List<ChangeRecord> changes;
    ...
    _changes.add(changes); // this will throw an error of mismatched types
}

The only thing you need to do to solve it is change this line:

bool deliverChanges() {
    List<ChangeRecord> changes;

to this:

bool deliverChanges() {
    List<C> changes;
@natebosch
Copy link
Member

I assume you're seeing the issue at runtime in DDC? what version of the SDK are you on?

@jodinathan
Copy link
Author

sorry.
DDC
dev-24 and dev-28 neither worked

@natebosch
Copy link
Member

If we change the type to List<C> then I think the assignment to ChangeRecord.ANY would fail.

https://github.com/dart-lang/observable/blob/bc4b2d9902099e459ea744b8969239156c65623f/lib/src/change_notifier.dart#L61

ChangeRecord.ANY is a List<ChangeRecord> which is not a List<C>

https://github.com/dart-lang/observable/blob/bc4b2d9902099e459ea744b8969239156c65623f/lib/src/records.dart#L22

I think this is likely to be a bit tougher to solve and I'm not sure if we have the bandwidth to dig deeply on it...

@jodinathan
Copy link
Author

yeah, just tested it and it threw an error with ANY.
that is probably why it was List<ChangeRecord> in the first place.

Maybe just sending an empty instead of the ANY const?
afterall, that dummy ChangeRecord wasn't much of a help.

Or you can store the last queue and send it again if the new queue is empty.

@kevmoo
Copy link
Collaborator

kevmoo commented Feb 23, 2018

CC @nshahan

@jodinathan
Copy link
Author

For now I am using my fork: https://github.com/jodinathan/observable
It basically changes:

https://github.com/dart-lang/observable/blob/bc4b2d9902099e459ea744b8969239156c65623f/lib/src/change_notifier.dart#L55

to:
List<C> changes;

and

https://github.com/dart-lang/observable/blob/bc4b2d9902099e459ea744b8969239156c65623f/lib/src/change_notifier.dart#L61

to:

changes = <C>[];

It is a breaking change, however, that empty ChangeRecord from ANY const has no value at all.

@shamblett
Copy link

Hi guys, any update on this as far as a permanent fix is concerned, I'd rather use observable if a fix is forthcoming.

@kevmoo
Copy link
Collaborator

kevmoo commented Aug 13, 2018

@nshahan ?

@nshahan
Copy link
Contributor

nshahan commented Aug 14, 2018

I'll do some testing on our codebase to see if we can easily fix up any breaks.

@buhhy
Copy link
Contributor

buhhy commented Dec 5, 2018

I've created a pull request to fix this issue:

#76

@wsakka
Copy link

wsakka commented Feb 23, 2019

Not sure why the flutter team leaves these issues open -- it's been a year, what's the status of this issue?

@jodinathan
Copy link
Author

@wsakka it has been fixed with version 0.22.1+5.
I am closing this issue.

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

No branches or pull requests

7 participants