Flutter 3.41 upgrade - #12
Open
xuala69 wants to merge 4 commits into
Open
Conversation
Reviewer's GuideUpgrades the package and example app to be compatible with Flutter 3.41 by updating Pdfrx integration, tightening SDK and Flutter version constraints, modernizing iOS embedding (AppDelegate, Podfile, Info.plist), and making a minor API cleanup in the page flip controller. Sequence diagram for updated Pdfrx cache and asset initialization in TurnablePdf.initPDFLoaderssequenceDiagram
participant App as App
participant TurnablePdf as TurnablePdf
participant Directory as Directory
participant Pdfrx as Pdfrx
participant RootBundle as rootBundle
App->>TurnablePdf: initPDFLoaders()
activate TurnablePdf
TurnablePdf->>Directory: Directory.systemTemp.path
TurnablePdf->>Directory: Directory(path: pdfrx_cache)
TurnablePdf->>Directory: exists()
alt cacheDir missing
TurnablePdf->>Directory: create(recursive: true)
end
TurnablePdf->>Pdfrx: cacheDirectoryPath ??= cacheDir.path
TurnablePdf->>Pdfrx: loadAsset ??= (String name) async { ... }
TurnablePdf->>RootBundle: load(name)
RootBundle-->>TurnablePdf: ByteData
TurnablePdf-->>Pdfrx: data.buffer.asUint8List()
deactivate TurnablePdf
Sequence diagram for updated iOS AppDelegate implicit engine initializationsequenceDiagram
actor User as User
participant UIApplication as UIApplication
participant AppDelegate as AppDelegate
participant EngineBridge as FlutterImplicitEngineBridge
participant PluginReg as GeneratedPluginRegistrant
User->>UIApplication: launch app
UIApplication->>AppDelegate: application(didFinishLaunchingWithOptions:)
activate AppDelegate
AppDelegate-->>UIApplication: super.application(...)
deactivate AppDelegate
UIApplication->>AppDelegate: didInitializeImplicitFlutterEngine(EngineBridge)
activate AppDelegate
AppDelegate->>PluginReg: register(with: EngineBridge.pluginRegistry)
deactivate AppDelegate
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
TurnablePdf.initPDFLoaders, you always create thepdfrx_cachedirectory even whenPdfrx.cacheDirectoryPathis already non-null; consider guarding the directory creation with aif (Pdfrx.cacheDirectoryPath == null)check to avoid unnecessary filesystem work. - In the new
example/ios/Podfile, you might want to uncomment and setplatform :ios, '13.0'(or whatever minimum you intend) so the iOS deployment target is explicit and aligned with Flutter 3.41’s requirements instead of relying on CocoaPods’ default.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TurnablePdf.initPDFLoaders`, you always create the `pdfrx_cache` directory even when `Pdfrx.cacheDirectoryPath` is already non-null; consider guarding the directory creation with a `if (Pdfrx.cacheDirectoryPath == null)` check to avoid unnecessary filesystem work.
- In the new `example/ios/Podfile`, you might want to uncomment and set `platform :ios, '13.0'` (or whatever minimum you intend) so the iOS deployment target is explicit and aligned with Flutter 3.41’s requirements instead of relying on CocoaPods’ default.
## Individual Comments
### Comment 1
<location path="lib/src/widgets/turnable_pdf.dart" line_range="237-243" />
<code_context>
- }
- return dir.path;
- };
+ final cacheDir = Directory(
+ '${Directory.systemTemp.path}${Platform.pathSeparator}pdfrx_cache',
+ );
+ if (!await cacheDir.exists()) {
+ await cacheDir.create(recursive: true);
+ }
+ Pdfrx.cacheDirectoryPath ??= cacheDir.path;
- Pdfrx.loadAsset = (String name) async {
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid unnecessary filesystem work when `Pdfrx.cacheDirectoryPath` is already configured.
This code creates/checks `cacheDir` on every `initPDFLoaders()` call, even when `Pdfrx.cacheDirectoryPath` is already set by the host. Consider guarding the directory creation with `if (Pdfrx.cacheDirectoryPath == null)` so you only construct `cacheDir` and hit the filesystem when no cache path is preconfigured, reducing unnecessary I/O for consumers who set this upfront.
</issue_to_address>
### Comment 2
<location path="example/ios/Podfile" line_range="1-2" />
<code_context>
+# Uncomment this line to define a global platform for your project
+# platform :ios, '13.0'
+
+# CocoaPods analytics sends network stats synchronously affecting flutter build latency.
</code_context>
<issue_to_address>
**suggestion (bug_risk):** It’s worth uncommenting and aligning the `platform :ios` declaration with Flutter’s minimum iOS version.
With this line commented out, CocoaPods falls back to the Xcode project deployment target, which may now diverge from Flutter’s minimum iOS version (especially since `MinimumOSVersion` was removed from `AppFrameworkInfo.plist`). That mismatch can lead to warnings and subtle build/runtime issues. Please set this `platform :ios` value explicitly to the minimum iOS version required by your Flutter SDK (e.g. `'13.0'` or higher).
```suggestion
# Define a global platform for your project aligned with Flutter's minimum iOS version
platform :ios, '13.0'
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+237
to
+243
| final cacheDir = Directory( | ||
| '${Directory.systemTemp.path}${Platform.pathSeparator}pdfrx_cache', | ||
| ); | ||
| if (!await cacheDir.exists()) { | ||
| await cacheDir.create(recursive: true); | ||
| } | ||
| Pdfrx.cacheDirectoryPath ??= cacheDir.path; |
There was a problem hiding this comment.
suggestion (performance): Avoid unnecessary filesystem work when Pdfrx.cacheDirectoryPath is already configured.
This code creates/checks cacheDir on every initPDFLoaders() call, even when Pdfrx.cacheDirectoryPath is already set by the host. Consider guarding the directory creation with if (Pdfrx.cacheDirectoryPath == null) so you only construct cacheDir and hit the filesystem when no cache path is preconfigured, reducing unnecessary I/O for consumers who set this upfront.
Comment on lines
+1
to
+2
| # Uncomment this line to define a global platform for your project | ||
| # platform :ios, '13.0' |
There was a problem hiding this comment.
suggestion (bug_risk): It’s worth uncommenting and aligning the platform :ios declaration with Flutter’s minimum iOS version.
With this line commented out, CocoaPods falls back to the Xcode project deployment target, which may now diverge from Flutter’s minimum iOS version (especially since MinimumOSVersion was removed from AppFrameworkInfo.plist). That mismatch can lead to warnings and subtle build/runtime issues. Please set this platform :ios value explicitly to the minimum iOS version required by your Flutter SDK (e.g. '13.0' or higher).
Suggested change
| # Uncomment this line to define a global platform for your project | |
| # platform :ios, '13.0' | |
| # Define a global platform for your project aligned with Flutter's minimum iOS version | |
| platform :ios, '13.0' |
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
1 participant
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.
Fix Pdfrx.getCacheDirectory (Issue) and upgrade to support Flutter 3.41
Summary by Sourcery
Upgrade the package and example app for compatibility with newer Flutter and iOS tooling while fixing PDF cache handling.
New Features:
Bug Fixes:
Enhancements:
Build: