refactor: federated plugin — Melos monorepo + flutter_pdfview_platform_interface - #360
Open
endigo wants to merge 4 commits into
Open
refactor: federated plugin — Melos monorepo + flutter_pdfview_platform_interface#360endigo wants to merge 4 commits into
endigo wants to merge 4 commits into
Conversation
Move the published package to packages/flutter_pdfview/ and turn the repository root into a Dart pub workspace driven by Melos 7. Nothing inside the package changes; this is layout only, so that platform implementations and a future web package can be split out without another repository move. - Root pubspec.yaml declares the workspace members and inlines the melos config (analyze / format / test / test:android / publish:dry-run scripts) - Members carry `resolution: workspace`; the single lockfile lives at the root and member lockfiles are untracked and gitignored - .gitignore patterns that were anchored to the old root (ios/.symlinks, example/android/app/.cxx, ...) are matched at any depth instead - publish.yml sets working-directory: packages/flutter_pdfview - scripts/ repointed at the new package location - CLAUDE.md documents the layout and drops paths that were stale from both this move and the earlier Kotlin/Swift migration (it still said .java/.m) Verified: 127 Dart tests pass, the Android Robolectric/JUnit suite builds green through the repointed script, `melos run analyze` and `format-check` are clean, and `flutter pub publish --dry-run` reports no content problems. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
Split the plugin into a federated structure. The shared value types, the settings/creation-param wire format and the method-channel plumbing move into a new flutter_pdfview_platform_interface package; PDFView now reaches the native views through FlutterPdfViewPlatform.instance. Adding a platform (web, macOS, Windows) no longer means touching the app-facing package — an implementation extends FlutterPdfViewPlatform and registers itself. What moved: - FitPolicy / PageAlignment / PdfColorMode / PDFPasswordFailure and every callback typedef, re-exported from flutter_pdfview so the public import path is unchanged - PdfViewSettings (was the private _PDFViewSettings), including updatesMap — the diff that updates a live view instead of remounting it - PdfViewCreationParams (was _CreationParams) - The platform-view construction: UiKitView, PlatformViewLink + initExpensiveAndroidView, and the unsupported-platform Text fallback - The per-view method channel, as MethodChannelPdfViewController What did not change: - The public Dart API. PDFView, PDFViewController, every callback and enum keep the same names, semantics and import path - The native code, the platform-view type and the method-channel protocol, so existing native implementations keep working PDFViewController is now a thin wrapper over PdfViewPlatformController. Its callbacks are a PdfViewCallbacks snapshot refreshed on every widget update rather than a live read of the widget, so the controller re-points them at construction and on each update — preserving the previous behavior that a rebuilt widget's callbacks take effect. Verified: the full 1.5.0-beta.10 Dart suite (127 tests) passes unmodified, plus 38 new tests for the interface covering the wire format, the PlatformInterface token check and the method-channel controller. Android Robolectric/JUnit green, analyze and format clean, and both packages pass `pub publish --dry-run` (the interface with zero warnings). Release order: flutter_pdfview_platform_interface must be published before flutter_pdfview, which depends on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
`melos bootstrap` writes a melos_<package>.iml next to each workspace member and one at the root. They are per-machine IDE files, not project config. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
…tlin
Two warnings that Flutter 3.44 prints on every build of an app using this
plugin. Neither was caused by the monorepo/federation work; both are the
plugin lagging behind toolchain changes.
iOS — Package.swift declared `dependencies: []`. Flutter 3.44+ requires
plugins to depend on FlutterFramework explicitly rather than inheriting the
embedder from the generated Runner package. Declared it and added the product
to the Swift target. The Objective-C shim only imports Foundation, so it
deliberately does not take the dependency.
Android — AGP 9 compiles Kotlin itself, and Flutter fails the build of any app
whose plugins also apply the Kotlin Gradle Plugin, so this plugin would have
broken apps on future Flutter releases. Applying KGP unconditionally causes
that; removing it unconditionally would break everyone on AGP 8, which this
plugin still supports (pubspec requires only Flutter >= 3.32). So it is now
applied only when AGP < 9, and the `kotlinOptions` block is replaced by
`kotlin { compilerOptions { jvmTarget } }`, which both toolchains understand.
Verified on the example app by temporarily moving it to AGP 9.3.1 + Gradle
9.5.0 + android.builtInKotlin=true: flutter_pdfview evaluates cleanly and
appears in no failure. The build does still fail there, but on packages
outside this repo that have not migrated — Flutter's own integration_test and
jni. The example app config was restored afterwards and is unchanged.
The KGP warning does not disappear on AGP 8, because the plugin still applies
KGP there by design. It goes away once an app is on AGP 9 with built-in Kotlin,
which is the case this change exists to keep working.
Also ignore SwiftPM .build/ output, which `pod lib lint` leaves behind.
Verified after the change: iOS builds with no FlutterFramework warning, iOS
integration tests 11/11 against real PDFKit, Android APK builds, Android unit
tests 123/0/0, Dart 127 + 38 pass, analyze clean, and `pod lib lint` still
passes so the CocoaPods path is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacks on #345 — base is
migrate/kotlin-swift, so review that one first.Turns the repository into a federated plugin, then fixes two toolchain warnings Flutter 3.44 prints on every build.
No public API change.
PDFView,PDFViewController, every callback and enum keep the same names, semantics and import path. The native code, the platform-view type and the method-channel protocol are untouched, so existing native implementations keep working — and the entire 1.5.0-beta.10 Dart suite passes unmodified.What changed
1. Melos + pub workspaces monorepo (
7ff312b)The published package moved to
packages/flutter_pdfview/. The root is now a workspace declaring members and inlining the melos config.melos run analyze | test | test:android | format | publish:dry-runcover every package.publish.ymlnow setsworking-directory: packages/flutter_pdfview, andscripts/was repointed.2. Extract
flutter_pdfview_platform_interface(2ebe1fd)FlutterPdfViewPlatformbuildViewreturns the hosting widgetPdfViewPlatformControllerPdfViewSettingsupdatesMap, the diff that updates a live view instead of remountingPdfViewCreationParams/PdfViewCallbacksMethodChannelFlutterPdfViewUiKitView,PlatformViewLink+initExpensiveAndroidView, unsupported-platformTextPDFViewControlleris now a thin wrapper overPdfViewPlatformController. Adding a platform (web #65, macOS #84, Windows #165) no longer means touching the app-facing package.The shared value types moved into the interface and are re-exported from
package:flutter_pdfview/flutter_pdfview.dart, so app code needs no changes.3. Toolchain fixes (
f16dc7b)iOS —
Package.swiftdeclareddependencies: []. Flutter 3.44+ requires plugins to declareFlutterFrameworkexplicitly instead of inheriting the embedder from the generated Runner package. Warning is gone. The ObjC shim only imports Foundation, so it deliberately does not take the dependency.Android — AGP 9 compiles Kotlin itself and Flutter fails the build of any app whose plugins also apply KGP, so this plugin would have broken apps on future Flutter releases. Removing KGP unconditionally would break everyone on AGP 8, which this plugin still supports (pubspec requires only Flutter ≥ 3.32), so it is now AGP-gated:
with
kotlinOptionsreplaced bykotlin { compilerOptions { jvmTarget } }, which both toolchains understand.Verification
pod lib lintmelos bootstrap/analyze/format-checkpub publish --dry-runThe AGP 9 path was verified rather than assumed, by temporarily moving the example to AGP 9.3.1 + Gradle 9.5.0 +
android.builtInKotlin=true. That caught a real bug — the naive form from the migration guide fails, because thekotlin { }extension does not exist under AGP 9 unless built-in Kotlin is enabled. With the fix,flutter_pdfviewevaluates cleanly and appears in no failure. The example app config was restored afterwards and is unchanged in this PR.Reviewer notes
flutter_pdfview_platform_interfacemust be published beforeflutter_pdfview, which declaresflutter_pdfview_platform_interface: ^1.0.0. Inside the workspace it resolves locally; on pub.dev it will not resolve until the interface is up.PdfViewCallbackssnapshot, re-pointed both at controller construction and on every widget update. Without that, a widget rebuilt before the platform view finished creating would dispatch into stale closures.flutter_pdfviewbumped to1.5.0-beta.11; the interface starts at1.0.0.Not covered
integration_test, andjni.🤖 Generated with Claude Code
https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F