fix(tvos_plugins): preserve existing .flutter-plugins-dependencies#15
Conversation
`ensureReadyForTvosTooling` was constructing a fresh `dependenciesJson` from scratch and writing it over the file that stock `flutter pub get` had just populated. The freshly written file kept only the `tvos` key and reset `dependencyGraph` to `[]`. That broke every later `_discoverTvosPlugins` call (notably the one inside `writeTvosDartPluginRegistrant` during the build pipeline) because the discovery walks `dependencyGraph` — empty graph → no plugins → empty `dart_plugin_registrant.dart`. Visible symptoms before this fix: federated tvOS plugins with `dartPluginClass:` (e.g. `shared_preferences_tvos`, `video_player_tvos`, `path_provider_tvos`) silently dropped from the registrant on every build, surfacing at runtime as `MissingPluginException` for legacy APIs or `Bad state: <X>Platform.instance must be set` for the newer async APIs. Integration test impact: `shared_preferences` went +85/-1 instead of +63/-23, `audioplayers` from a hang to +53/-1, `flutter_secure_storage` and `video_player` from fail/timeout to fully green. The fix reads the existing file (falling back to a fresh skeleton if absent or malformed) and only grafts the tvOS plugin entries onto the `plugins` map. iOS/Android/macOS/web entries and the `dependencyGraph` written by stock pub get are preserved untouched. Function is tvOS-scoped via the early `if (!tvosDir.existsSync()) return` at the top, so non-tvOS projects see no behavioural change.
| // Malformed file — fall back to a fresh skeleton (defined above). | ||
| } on FileSystemException { | ||
| // File disappeared between existsSync() and read — same fallback. | ||
| } |
There was a problem hiding this comment.
json.decode(...) as Map<String, dynamic> throws TypeError (not FormatException) when the file contains valid JSON whose root is not an object — e.g. [], null, or a bare number. TypeError is not caught by either branch here, so it propagates out of ensureReadyForTvosTooling and crashes the build with a raw Dart stack trace.
The existing _walkPluginDependencies reader (same file, ~line 105) already guards against this. Same fix here:
try {
final decoded = json.decode(depsFile.readAsStringSync());
if (decoded is Map<String, dynamic>) {
dependenciesJson = decoded;
} else {
globals.logger.printWarning(
'.flutter-plugins-dependencies is not a JSON object; regenerating from scratch.',
);
}
} on FormatException catch (e) {
globals.logger.printWarning(
'.flutter-plugins-dependencies contains malformed JSON ($e); regenerating from scratch.',
);
} on FileSystemException catch (e) {
globals.logger.printWarning(
'.flutter-plugins-dependencies disappeared before it could be read ($e); regenerating from scratch.',
);
}Also worth adding a printWarning to the FormatException/FileSystemException branches — silent swallowing of a corrupt file makes debugging hard.
There was a problem hiding this comment.
Good catch — fixed in edb2d98 with the type-check pattern you suggested, plus the printWarning for both FormatException and FileSystemException branches. Silent fallback was indeed making the original bug hard to debug.
| dependenciesJson['plugins'] = pluginsMap; | ||
|
|
||
| final pluginsBuffer = StringBuffer(); | ||
|
|
There was a problem hiding this comment.
Same TypeError risk here: if the file was read successfully but plugins is not a Map (e.g. "plugins": []), the as Map<String, dynamic>? cast throws outside the try/catch and crashes.
Safer:
final rawPlugins = dependenciesJson['plugins'];
final pluginsMap = rawPlugins is Map<String, dynamic>
? rawPlugins
: <String, dynamic>{};There was a problem hiding this comment.
Same pattern applied in edb2d98 — rawPlugins is Map<String, dynamic> ? rawPlugins : <String, dynamic>{}. Also fixed an identical latent TypeError you mentioned was guarded in _walkPluginDependencies line 105 but actually wasn't — the regression test for non-object root caught it.
|
Overall: fix is correct in intent, two implementation issues flagged inline. Missing test: the core invariant — that
|
…on tests Addresses three review items on PR fluttertv#15: 1. `ensureReadyForTvosTooling` — replace the blind `as Map<String, dynamic>` cast on the parsed `.flutter-plugins-dependencies` with a runtime type-check. A valid-JSON-but-non-object root (`[]`, `null`, a bare number) would otherwise throw `TypeError` outside the existing `FormatException`/`FileSystemException` handlers and crash the build. Same change applied to the `dependenciesJson['plugins']` cast: a wrong-shaped `plugins: []` value now falls back to an empty map instead of throwing. 2. The two `FormatException` / `FileSystemException` branches now emit `printWarning` messages explaining the fallback. Silent swallowing was making the original `dependencyGraph`-wipe bug nearly impossible to debug. 3. `_walkPluginDependencies` had the same latent `TypeError` (line 105) — caught by the new regression tests while writing them. Fixed with the same type-check pattern; the regression tests would have failed without it. Adds three test cases under the existing `ensureReadyForTvosTooling end-to-end` group: - preserves dependencyGraph + ios/android plugin keys (the core regression test the reviewer asked for — writes a realistic pub-get-shaped file, calls the function, asserts everything except the `tvos` graft is bit-identical) - falls back when root JSON is not an object (e.g. `[]`) - falls back when `plugins` key is the wrong shape (e.g. an array) All 29 tests in tvos_plugins_test.dart pass.
|
Thanks for the careful review — all three items addressed in edb2d98:
All 29 tests in |
DenisovAV
left a comment
There was a problem hiding this comment.
All issues addressed. TypeError handled with type guard, pluginsMap extraction safe, regression tests added. LGTM.
Summary
Fixes a regression in
ensureReadyForTvosToolingwhere the CLI was wiping.flutter-plugins-dependenciesinstead of grafting tvOS entries onto it. Every federated tvOS plugin withdartPluginClass:was silently dropped from the generateddart_plugin_registrant.dart, breaking the federated plugin chain at runtime.The bug
ensureReadyForTvosToolingconstructed a freshdependenciesJsonand wrote it over the file that stockflutter pub gethad just populated:That wiped the
ios/android/macos/webplugin lists and thedependencyGraphwritten by stock pub get.Later in the build pipeline,
writeTvosDartPluginRegistrantcalls_discoverTvosPluginsagain, which walksdependencyGraphto find tvOS plugins. With the graph emptied, it finds nothing → writes an emptydart_plugin_registrant.dart→ federated plugins never call theirregisterWith().Visible symptoms
shared_preferences_tvosBad state: The SharedPreferencesAsyncPlatform instance must be set.video_player_tvosUnimplementedErroronVideoPlayerPlatform.instance.create(...)path_provider_tvosMissingPluginException(...path_provider...)(transitive via other plugins)audioplayers_tvosMissingPluginException(...xyz.luan/audioplayers.global...)flutter_secure_storage_tvosEach of these declares a
dartPluginClass:(e.g.SharedPreferencesFoundation,AVFoundationVideoPlayer,PathProviderTvos). Plugins that only declarepluginClass:(nodartPluginClass:) were unaffected because their registration goes through the SwiftGeneratedPluginRegistrant.swiftpath, which has a separate generator that doesn't depend ondependencyGraph.The fix
Read the existing
.flutter-plugins-dependencies(falling back to a fresh skeleton if absent or malformed) and only graft the tvOS plugin entries onto thepluginsmap.ios/android/macos/webentries and thedependencyGraphwritten by stock pub get are preserved.Scope / cross-platform safety
ensureReadyForTvosToolingearly-returns on line 280 if notvos/directory exists:So projects without a tvOS target (pure iOS / Android / web / macOS / Linux / Windows) hit the early return and see zero behavioural change. The fix is strictly additive for tvOS projects — it preserves data that was previously being thrown away.
Test plan
Tested against
fluttertv/pluginsintegration suite (11 plugins) with Flutter 3.44.0:Newly passing integration:
flutter_secure_storage_tvos,video_player_tvos.Massively improved (failures now visible as real plugin bugs rather than masked by registration errors):
shared_preferences_tvos: was+63 -23→ now+85 -1audioplayers_tvos: was hung at+2 -40→ now+53 -1The 2 remaining failures are real plugin logic bugs unrelated to this fix (a migration tool ordering bug in shared_preferences and one audioplayers test).
Related
Pairs with
fluttertv/plugins@48c967ewhich untracks the stale emptyGeneratedPluginRegistrant.swiftstubs from each plugin's example app. Together these two commits fully restore federated tvOS plugin registration on Flutter 3.44.