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

Create DevTools extension for package:provider #832

Merged
merged 25 commits into from
Sep 24, 2023

Conversation

kenzieschmoll
Copy link
Contributor

@kenzieschmoll kenzieschmoll commented Aug 18, 2023

This sets up the skeleton for the DevTools extension though and moves the provider screen code from flutter/devtools to the provider repo.

New: Provider extension supplied by package:provider:

Screenshot 2023-08-18 at 2 28 23 PM

Old: Provider screen implemented as part of DevTools:

Screenshot 2023-08-18 at 2 28 37 PM

Draft for #833

@rrousselGit
Copy link
Owner

Thanks for this! Is there anything specific I should look into? Maybe I'm expected to debug the error?

@kenzieschmoll
Copy link
Contributor Author

kenzieschmoll commented Aug 28, 2023

I just updated #833 (comment) with an update on this work. With the latest commit, you should be able to pull down this PR and build in the simulated DevTools environment.

AIs:

  1. Yes, if you could help with debugging the error and verifying this extension is WAI, that would be great.
  2. Recommended: restructure the provider repo as specified by https://pub.dev/packages/devtools_extensions#where-to-put-your-source-code. I partially did this in this PR but I didn't move all the existing top-level content into provider/packages/provider to keep the diff manageable.
  3. Add testing: rewrite provider_integration_test.dart as a true integration test, and ensure the provider_screen_test that we originally had in DevTools works here (this test likely needs to be re-written so that it can run on the VM, or it will have to be run on the chrome test target).
  4. I also noticed that clicking the "settings" button on the provider screen throws an exception.

The failure seems flaky - maybe due to hot restart? On a fresh build of an app that uses package:provider (gallery) and the first load of the extension in the simulated devtools environment, I see the intended behavior:
Screenshot 2023-08-28 at 3 15 35 PM

Eager to hear your feedback on the development workflow and any feature requests.

@rrousselGit
Copy link
Owner

Thanks for this!

Would it make sense & be reasonable to publish dev release of the extension with the current state of things? (after I've had the time to play around with it a bit :) )

@kenzieschmoll
Copy link
Contributor Author

Would it make sense & be reasonable to publish dev release of the extension with the current state of things? (after I've had the time to play around with it a bit :)

Right now the extension support in DevTools is behind an experiments flag (hoping to turn that on by end of september), so even if you publish a dev version of provider with the extension, you wouldn't be able to load it in production DevTools (i.e. you'd still have to build DevTools locally with experiments enabled to see your extension in a real DevTools instance).

We are also changing the config.json spec to be a config.yaml file (flutter/devtools#6280), but I expect that change to land this week. All this to say, there is nothing that stops you from landing and publishing a dev version, but there may be some more breaking changes coming in the next few weeks. So my recommendation would be to hold off on publishing for a week or two.

@rrousselGit
Copy link
Owner

Great, sounds good to me. Thanks for sharing :)

@kenzieschmoll
Copy link
Contributor Author

kenzieschmoll commented Sep 14, 2023

Hi @rrousselGit this is ready to go. With the latest commit, AIs 1) and 4) above should be resolved. I fixed 4) and I just haven't been able to reproduce 1). AIs 2) and 3) still stand.

The latest versions of devtools_extensions and devtools_app_shared have been published. Expect some polish to come over the next couple weeks but these are safe to start using. The latest commit I pushed up here bumps the pubspec to use the latest published versions. DevTools extensions support will be landed in the SDK shortly (I expect it to get to Flutter master within a couple days).

Until then, you can develop your extension locally with the Simulated DevTools environment, or build DevTools locally to test your extension against a real DevTools instance. Instructions for both are here: https://pub.dev/packages/devtools_extensions#dart--flutter-devtools-extensions.

Please let me know if you have any questions or how I can help. Thanks!

Screenshot 2023-09-14 at 3 40 35 PM

@kenzieschmoll kenzieschmoll marked this pull request as ready for review September 14, 2023 23:38
@kenzieschmoll kenzieschmoll changed the title [WIP] Create DevTools extension for package:provider Create DevTools extension for package:provider Sep 14, 2023
@kenzieschmoll
Copy link
Contributor Author

@rrousselGit do you want to take this PR over? or do you want me to land it as, and then you can address 2) and 3) in a follow up?

@rrousselGit
Copy link
Owner

I was thinking about merging it as is, and dealing with tests/folders separately on my own.

Is it fine to make a dev release with this now? I assume we could make a dev release even if it isn't on Flutter's master channel. And it'd be accessible as soon as this lands on Flutter's side

@kenzieschmoll
Copy link
Contributor Author

I was thinking about merging it as is, and dealing with tests/folders separately on my own.

Is it fine to make a dev release with this now? I assume we could make a dev release even if it isn't on Flutter's master channel. And it'd be accessible as soon as this lands on Flutter's side

Yes it is okay to make a dev release now. This plan SGTM. The extension support should be on Flutter master, and a more polished version of the extension support will be on master by the end of this week (I fixed a few bugs over the last few days I will release into the Dart SDK shortly).

@rrousselGit rrousselGit merged commit a689f40 into rrousselGit:master Sep 24, 2023
3 checks passed
@rrousselGit
Copy link
Owner

Alright, thanks for this!

I've made a dev release (6.1.0-dev.0) with those changes.

@rrousselGit
Copy link
Owner

Ah, I gitignored the build folder. But it looks like this ignores it from pub publish too.
I've added a pubignore to circumvent this. It's likely worth mentioning in the docs next to the bit about not commiting build

@rrousselGit
Copy link
Owner

Looks like all is well now. I've managed to use the extension on the example app using the published Provider version.

That's awesome!

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.

3 participants