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

Add riverpod tool #4210

Closed
wants to merge 8 commits into from
Closed

Conversation

adsonpleal
Copy link

This PR adds support to Riverpod, the code is heavily based on the current Provider implementation. It uses provider's InstanceViewer widget to show the state values.

Riverpod-devtools.mp4

The discussion about this solution can be found at riverpod's repo.

TODO: this PR is still a Draft, the tests will be added before opening it for real.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@jacob314
Copy link
Contributor

Fyi @rrousselGit.

@@ -9,7 +9,8 @@ environment:
dependencies:
flutter:
sdk: flutter
flutter_riverpod: ^2.0.0-dev.5
flutter_riverpod:
path: /Users/adson.leal/dev/personal/riverpod/packages/flutter_riverpod # TODO: need to use an actual version
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rrousselGit we need to release a version first, before merging this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a problem. It'll happen later

@adsonpleal
Copy link
Author

@rrousselGit Just added tests, this PR is ready to be reviewed, but as we need the riverpod one to be merged first I'll leave this one as draft for now.

@rrousselGit
Copy link
Contributor

That's great, thanks for your work!

I'll try playing around with it this w-e to see.

@rrousselGit
Copy link
Contributor

This is looking great to me.

Any thoughts @jacob314?

@rrousselGit
Copy link
Contributor

If that's good for you, I can merge the associated PR adding dart:developer events to Riverpod, then we can remove the git dependency.

@rrousselGit
Copy link
Contributor

While we're waiting for a review, it'd be great to add support for the upcoming code-generator

Fancy supporting it?

@adsonpleal
Copy link
Author

You mean Riverpod 2.0?

@rrousselGit
Copy link
Contributor

I mean the "Riverpod_generator" project in the master branch of Riverpod

@adsonpleal
Copy link
Author

I'll take a look, if I have questions I'll talk to you.

@cedvdb
Copy link

cedvdb commented Nov 17, 2022

I'm confused why this PR is made on the devtool repository itself as riverpod has nothing to do with the dev tools.

I'd have assumed a riverpod_devtools or some extension package. Same if I want to create my own dev tool extension.

Edit: oh right, it does not support extensions yet. In any case I don't think public libraries should be accepted here, even widely used ones.

@kenzieschmoll
Copy link
Member

Since DevTools does not yet support plugins or extensions, we're going to close this for now. DevTools plugin support is something we are hoping to work on later this year (here is an existing design proposal for this). Converting the existing package:provider screen as well as this riverpod screen to a plugin would be great first use cases to work on when plugin support is added. We will definitely reach out when this support is available. Thank you for the contribution!

@kenzieschmoll
Copy link
Member

Hello @adsonpleal and @rrousselGit - now that support for DevTools extensions has officially landed, do you have interest in re-landing this PR on the riverpod repo directly as a DevTools extension? As an example, the Provider related code from DevTools was migrated to a DevTools extension in package:provider here, and published as part of 6.1.0-dev.1

@rrousselGit can you provide some guidance on which riverpod package the extension should live in (I would guess riverpod?)

@rrousselGit
Copy link
Contributor

I'm already working on it :)

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

Successfully merging this pull request may close these issues.

5 participants