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

Editing value on provider throws eval error #3003

Closed
kenzieschmoll opened this issue May 11, 2021 · 6 comments · Fixed by #3010
Closed

Editing value on provider throws eval error #3003

kenzieschmoll opened this issue May 11, 2021 · 6 comments · Fixed by #3010

Comments

@kenzieschmoll
Copy link
Member

Repro steps:

  1. run the flutter gallery app
  2. Click the "Reply" app from the carousel of apps at the top of the flutter gallery homescreen
  3. On the provider page of DevTools, the EmailStore provider will become available
  4. try to edit the _count field

Screen Shot 2021-05-11 at 2 06 06 PM

Screen Shot 2021-05-11 at 2 06 12 PM

@rrousselGit @jacob314

@rrousselGit

This comment has been minimized.

@rrousselGit
Copy link
Contributor

rrousselGit commented May 12, 2021

Oh, I found the problem.

It's because EmailStore is defined as:

class EmailStore with ChangeNotifier

rather than:

class EmailStore extends ChangeNotifier

That messes up with the informations about where _count is coming from.

With mixins, _count's Instance.owner is

Instance(name: _EmailStore&Object&ChangeNotifier, uri: package:gallery/studies/reply/model/email_store.dart)

rather than:

Instance(name: ChangeNotifier, uri: package:flutter/.../foundation.dart)

That's a problem because since it is a private property, the eval needs to use the library that defines the property for the mutation to work.
But with mixins, vm_service gives an incorrect uri.

So it's behaving as if email_store.dart tried to update _count instead of as if flutter/foundation.dart tried to update it. But the former is not legal, hence the error.

I don't think this can be fixed. At least not without vm_service giving us the correct uri.

@rrousselGit
Copy link
Contributor

Mixins also are a source of problem with #3006 / #3004 by the way, for the same reasons.

#3006 relies on checking whether the property's owner URI matches with the name of the inspected application.
But since with mixins, ChangeNotifier._count is said to be defined by the class that mixes-in ChangeNotifier, then the property is not considered as "defined by a dependency", so it is visible.

@jacob314
Copy link
Contributor

Until we get the mixin issues sorted out how about we disable edits on private properties to be safe. I'd also be happy with disabling edits on all properties by default and only showing it as "experimental: edit property values" in the settings page.

@kenzieschmoll
Copy link
Member Author

Seeing this issue again with a different use case. Here I am trying to edit the _value of a ValueNotifier field
Screen Shot 2021-05-13 at 3 06 27 PM
doing so yields this error:
Screen Shot 2021-05-13 at 3 06 33 PM

Repro steps:

  1. run DevTools (A) on the web (this is the test app) *pro tip: change this to light theme so there is no confusion about which app is the debugger and which app is being debugged.
  2. Connect this instance of DevTools to an arbitrary flutter app (this is so that the PerformanceController will show up in the list of providers).
  3. run another instance of DevTools (B)(desktop or web, whichever you prefer).
  4. Connect this DevTools to the DevTools web app (A) you launched in step 1).
  5. Navigate to the provider page in DevTools (B). You should see the PerformanceController in the list of providers. Try to change the value of the matchIndex notifier as the screenshot shows.

@elliette
Copy link
Member

Now that the Provider panel is a DevTools extension, I'm closing this issue in favor of rrousselGit/provider#883.

@elliette elliette closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants