Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial version of package:extension_discovery #129
Initial version of package:extension_discovery #129
Changes from 5 commits
b3c6440
34a3c82
8bfd6e4
ed9278c
626b3c5
b6a0dd4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add here that it is also the responsibility of the package that will be extended to detect when the cache has been invalidated so that they can call
findExtensions
again to get an updated set of extensions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If people want to watch the file-system for changes, then I think it makes sense to build it in here.
You'd need to watch
.dart_tool/package_config.json
andconfig.json
for every mutable extension.I imagine that devtools will load extensions when a debugging instance is launched. If I do
dart pub get
after launching my app, can I expect hotreloading of all dependencies to work?I don't mind adding some watch logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding some watch logic would be great. In the short term, we can probably get by with having a way for the user to manually refresh the list of extensions from the extensions settings menu and by refreshing the list of available extensions on each hot reload or restart event. This of course would only be possible for Flutter. If there was a running dart CLI app that DevTools was connected to, the app would need to be killed, restarted, and reconnected to DevTools to pick up any changes that happened from running
dart pub get
.@christopherfujino do you know if all the dependencies are reloaded on hot reload? If we can expect this to work, DevTools can automatically refresh the list of extensions on hot reload / restart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just noting that this will not apply for DevTools extensions since we are not shipped as a pub package. Not sure if there are other "non-package" use cases for extensions, or if DevTools is the exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's recommendation because it really depends on the use-case.
In many cases if you're writing an extension to a package, then you're implementing an interface from the package, so then it's also natural to have a dependency on it.
If you wanted to have to ability to do, you could ask devtools extension packages to have a dependency on
devtools_ext
and then bump that package when you break the extension interface.But it's also fair that you just detect that an extension isn't compatible and thus shouldn't be loaded.
In you use-case, not loading the extension is probably better than having a version conflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one caveat is that a profile mode flutter app is AOT compiled but findExtensions would still work if given the location of this app's package_config.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I just think it's important to point out that if you are writing a Flutter app, you can't really use
findExtensions
and have it magically working on your phone.Because obviously the
package_config.json
and all the source code for the dependencies installed in pub-cache, aren't available on your phone, at-least not without some serious hacks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think most of my comments are because I'm reading from the perspective of having another tool (DevTools, which is running on the user's local machine) call findExtensions on a separate app (also running on the user's machine), and the warnings you are heading are when an app would be trying to call findExtensions itself.