Repository navigation
[flutter_tools] Implement generic extension protocol base and isolate runner - #190734
auto-submit[bot] merged 6 commits into
Conversation
github-actions
Bot
added
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pub workspace architecture and protocol implementation for Flutter Tools extensibility, including core models, isolate-based RPC transport, and host-side management via ExtensionManager and ExtensionDiscovery. Feedback focuses on improving robustness and preventing resource leaks, specifically by handling individual spawn failures in spawnAll, ensuring isolates are killed during disposal even if peer closure fails, validating the handshake response type, checking for duplicate RPC method registrations, logging RPC handler exceptions within the isolate, and using eager list copying instead of lazy casting during deserialization.
f2c4ad5 to
ba000ed
Compare
ba000ed to
dd28bb0
Compare
fe2d124 to
defa25b
Compare
defa25b to
ec51621
Compare
| beta: FeatureChannelSetting(available: true), | ||
| stable: FeatureChannelSetting(available: true), |
There was a problem hiding this comment.
Since this feature isn't fully implemented yet, should we make it unavailable on beta and stable? I don't feel strongly about this.
| _featureFlags = featureFlags ?? context.get<FeatureFlags>()!; | ||
|
|
||
| /// The active host operating system platform (e.g. `'linux'`, `'macos'`, `'windows'`). | ||
| final String hostPlatform; |
There was a problem hiding this comment.
Can we plumb enums around and convert to strings right before entering the protocol layer? It's very easy to mix up strings and cause unintended values to be passed around. Strong types would help us prevent mistakes.
Alternatively, could we add a platform enum to flutter_tools_extension?
| final services = servicesList is List ? List<String>.from(servicesList) : <String>[]; | ||
| final Object? platformsList = json['supportedPlatforms']; | ||
| final supportedPlatforms = platformsList is List | ||
| ? List<String>.from(platformsList) |
There was a problem hiding this comment.
Should we lower case these platform strings, to mirror how supportsHostPlatform lower cases strings? (If we switch to enums, that'd address this concern)
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
This looks good to me modulo nitpicks!
flutter-dashboard
Bot
removed
the
autosubmit
2 participants
Part of the tool extensibility prototype effort.
Implements the generic JSON-RPC protocol framing, capability handshake, and isolate runner infrastructure for Flutter tool extensions:
ToolExtensionEntryPoint,ToolExtensionService,ToolExtensionRunnerinpackage:flutter_tools_extension.ExtensionManagerandExtensionDiscoveryinpackage:flutter_tools.Part of #190746
Part of #190755
Pre-launch Checklist
///).