-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/upgrade flutter sdk 3.29.0 #27
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces extensive modernization and refactoring across both the Android/iOS build systems and the Flutter/Dart codebase. The Android Gradle scripts adopt the plugins DSL, update SDK and dependency versions, and streamline configuration loading. The iOS Podfile updates Firebase Firestore. In Dart, a sweeping change converts many model and state classes to Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Widget
participant PagingController
participant ViewModel
UI->>PagingController: Provide fetchPage callback
PagingController->>ViewModel: fetchPage(pageKey, isInitialLoad)
ViewModel->>PagingController: Return user list (paged data)
PagingController-->>UI: Notify with new paging state
UI->>PagingController: Call refresh() (on pull-to-refresh)
PagingController->>ViewModel: fetchPage(firstPageKey, true)
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/ui_kit/paged_view/controller/common_paging_controller.dart (1)
51-63
: 🛠️ Refactor suggestionDirectly mutating
pagingController.items
breaks immutability contract
PagingController.items
is documented as read-only, and in v5 it returns anUnmodifiableListView
. Modifying it viainsert
,removeAt
, etc. relies on internal implementation details and may throw or silently do nothing in future releases.Prefer emitting a new list through
pagingController.value = pagingController.value.copyWith(itemList: newList)
:- _pagingController.items?.insert(index, item); - pagingController.notifyListeners(); + final current = _pagingController.itemList ?? <T>[]; + final updated = [...current]..insert(index, item); + _pagingController.value = _pagingController.value.copyWith(itemList: updated);Repeat the same pattern for
insertAllItemsAt
,updateItemAt
,removeItemAt
,removeRange
, andclear
.Also applies to: 69-87
🧹 Nitpick comments (5)
lib/ui_kit/paged_view/controller/common_paging_controller.dart (1)
24-27
: Setter mutates controller state each time an error is assigned
_pagingController.value = _pagingController.value.copyWith(error: appException)
creates a newPagingState
on every assignment. That is correct, but:
- If the same
appException
is re-set repeatedly (e.g., during retries) the widget tree will still rebuild each time – possibly unnecessarily.- Consider short-circuiting when the new error equals the current one to avoid redundant rebuilds.
lib/model/base/paged_list.dart (1)
19-20
: Consider addressing the existing TODO commentThere's an existing TODO comment that appears unrelated to the current changes but might be worth addressing in a future update.
- // TODO(minh): fix depend on project #0 + // TODO: Fix isLastPage logic to properly check for actual last page conditionsuper_lint/pubspec.yaml (1)
11-11
: Remove trailing whitespaceThere are trailing spaces on this line that should be removed to comply with YAML style guidelines.
- analyzer: 6.11.0 + analyzer: 6.11.0🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 11-11: trailing spaces
(trailing-spaces)
super_lint/lib/src/index.dart (1)
24-24
: Commented out export lineThis commented export line appears to be preparation for a future change. Consider either uncommenting it if needed or removing it entirely to avoid confusion.
pubspec.yaml (1)
63-63
: Remove trailing spaces.There are trailing spaces on these lines that should be removed for consistent formatting.
- isar: + isar: - isar_generator: + isar_generator:Also applies to: 96-96
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 63-63: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
libisar.dylib
is excluded by!**/*.dylib
📒 Files selected for processing (55)
android/app/build.gradle
(2 hunks)android/build.gradle
(1 hunks)android/gradle.properties
(1 hunks)android/gradle/wrapper/gradle-wrapper.properties
(1 hunks)android/settings.gradle
(1 hunks)ios/Podfile
(1 hunks)ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
(1 hunks)lib/model/api/api_refresh_token_data.dart
(1 hunks)lib/model/api/api_user_data.dart
(1 hunks)lib/model/api/base/data_response.dart
(4 hunks)lib/model/api/base/records_response.dart
(1 hunks)lib/model/api/base/results_response.dart
(1 hunks)lib/model/api/mock/mock_data.dart
(2 hunks)lib/model/api/server_error.dart
(1 hunks)lib/model/api/server_error_detail.dart
(1 hunks)lib/model/base/load_more_output.dart
(1 hunks)lib/model/base/paged_list.dart
(1 hunks)lib/model/firebase/firebase_conversation_data.dart
(1 hunks)lib/model/firebase/firebase_conversation_user_data.dart
(1 hunks)lib/model/firebase/firebase_message_data.dart
(1 hunks)lib/model/firebase/firebase_reply_message_data.dart
(1 hunks)lib/model/firebase/firebase_user_data.dart
(1 hunks)lib/model/other/app_notification.dart
(1 hunks)lib/model/other/initial_resource.dart
(1 hunks)lib/ui/base/common_state.dart
(1 hunks)lib/ui/page/all_users/all_users_page.dart
(1 hunks)lib/ui/page/all_users/view_model/all_users_state.dart
(1 hunks)lib/ui/page/chat/view_model/chat_state.dart
(1 hunks)lib/ui/page/contact_list/view_model/contact_list_state.dart
(1 hunks)lib/ui/page/home/home_page.dart
(2 hunks)lib/ui/page/home/view_model/home_state.dart
(1 hunks)lib/ui/page/home/view_model/home_view_model.dart
(1 hunks)lib/ui/page/login/login_page.dart
(1 hunks)lib/ui/page/login/view_model/login_state.dart
(1 hunks)lib/ui/page/main/view_model/main_state.dart
(1 hunks)lib/ui/page/register/register_page.dart
(1 hunks)lib/ui/page/register/view_model/register_state.dart
(1 hunks)lib/ui/page/rename_conversation/view_model/rename_conversation_state.dart
(1 hunks)lib/ui/page/setting/view_model/setting_state.dart
(1 hunks)lib/ui_kit/common_scaffold.dart
(1 hunks)lib/ui_kit/paged_view/common_paged_grid_view.dart
(1 hunks)lib/ui_kit/paged_view/common_paged_list_view.dart
(3 hunks)lib/ui_kit/paged_view/common_paged_sliver_grid.dart
(1 hunks)lib/ui_kit/paged_view/common_paged_sliver_list.dart
(1 hunks)lib/ui_kit/paged_view/controller/common_paging_controller.dart
(5 hunks)pubspec.yaml
(2 hunks)super_lint/example/pubspec.yaml
(1 hunks)super_lint/lib/src/base/rule_config.dart
(1 hunks)super_lint/lib/src/index.dart
(2 hunks)super_lint/lib/src/utils/lint_extensions.dart
(1 hunks)super_lint/pubspec.yaml
(1 hunks)test/unit_test/ui/page/all_users/view_model/all_users_view_model_test.dart
(1 hunks)test/unit_test/ui/page/home/view_model/home_view_model_test.dart
(6 hunks)test/unit_test/ui/page/main/view_model/main_view_model_test.dart
(5 hunks)test/widget_test/ui/page/home/home_page_test.dart
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
super_lint/pubspec.yaml
[error] 11-11: trailing spaces
(trailing-spaces)
pubspec.yaml
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 96-96: trailing spaces
(trailing-spaces)
🔇 Additional comments (81)
super_lint/example/pubspec.yaml (2)
9-9
: Analyzer version upgrade looks goodThe update to analyzer 7.3.0 in dependency_overrides is consistent with the Flutter SDK 3.29.0 upgrade objectives.
16-16
: Custom lint package update is appropriateUpdating to custom_lint 0.7.5 aligns with the analyzer upgrade and ensures compatibility with the latest tooling.
ios/Podfile (1)
37-37
: FirebaseFirestore iOS SDK update looks goodUpdating to FirebaseFirestore iOS SDK 11.8.0 aligns with the broader Firebase dependency updates in the project. This helps maintain cross-platform consistency.
android/gradle.properties (2)
1-1
: Increased Gradle memory allocation is appropriate but noteworthyThe JVM arguments have been significantly enhanced:
- Increasing max heap from the previous setting to 4G
- Setting max metaspace size to 2G
- Adding heap dumps on OOM errors
These changes will improve build performance and stability, particularly for larger projects, but may require more system resources.
4-4
: Kotlin JVM target validation disabledAdding
kotlin.jvm.target.validation.mode=IGNORE
disables JVM target compatibility verification during the build. This is appropriate when upgrading Gradle and avoiding false positives from the Kotlin compiler.android/gradle/wrapper/gradle-wrapper.properties (1)
5-5
: Appropriate Gradle upgrade to version 8.3Updating to Gradle 8.3 is a significant but necessary upgrade as part of the Flutter SDK 3.29.0 migration. This version provides improved build performance and compatibility with newer Android tooling.
Verify that all team members have JDK 17 or higher installed, as Gradle 8.x requires it.
lib/ui_kit/common_scaffold.dart (1)
47-47
: Appropriate update for deprecated opacity methodReplacing
withOpacity(0.6)
withwithValues(alpha: 0.6)
correctly addresses the deprecated opacity usage mentioned in the PR objectives. This matches the Flutter 3.29.0 recommended approach.lib/model/api/server_error.dart (1)
8-8
: Good use of sealed class for type safetyUpgrading to a sealed class is a good practice when working with freezed-generated code in newer Flutter versions. This restricts subclassing to the current library, providing better type safety and alignment with modern Dart practices.
lib/model/api/base/results_response.dart (1)
7-7
: Consistent use of sealed class with genericsConverting to a sealed class for this generic response type maintains consistency with the project-wide refactoring pattern. This is appropriate for freezed-generated code with generics and provides better encapsulation.
lib/model/api/api_user_data.dart (1)
9-9
: Appropriate sealing of user data modelConverting ApiUserData to a sealed class is appropriate for user-related data models, ensuring controlled inheritance. This change maintains consistency with the codebase-wide refactoring pattern while enhancing data integrity.
lib/ui/page/register/register_page.dart (1)
185-185
: Updated to non-deprecated opacity APIGood update from the deprecated
withOpacity
to the newerwithValues(alpha:)
method. This change maintains the same visual behavior (fully opaque when enabled, 50% opaque when disabled) while addressing deprecation warnings in the new Flutter SDK.lib/ui/base/common_state.dart (1)
8-8
: Enhanced type safety for state managementMaking
CommonState<T>
a sealed class is particularly valuable for state management patterns. This change ensures that all possible state variations must be defined in this library, which facilitates exhaustive pattern matching when handling different states in UI components. The combination of sealed class, freezed annotation, and generic type parameter creates a robust and flexible state management foundation.lib/model/other/app_notification.dart (1)
6-6
: Good upgrade to sealed class for AppNotificationThe change from standard class to sealed class is appropriate for
freezed
-generated code and aligns with the Flutter SDK 3.29.0 upgrade. Sealed classes provide better type safety by restricting inheritance to the current library and enabling exhaustive pattern matching, which is particularly beneficial withfreezed
union types.lib/ui/page/register/view_model/register_state.dart (1)
8-8
: Good conversion to sealed class for state managementConverting
RegisterState
to a sealed class improves type safety and better supports pattern matching for state handling. This change is consistent with modern Dart practices for immutable state objects and aligns well with the freezed package's intended usage patterns.lib/ui/page/contact_list/view_model/contact_list_state.dart (1)
8-8
: Appropriate upgrade to sealed classThe change to a sealed class for
ContactListState
is beneficial for state management consistency and type safety. This modification fits well with the freezed pattern and ensures that all possible state variations are known at compile time.lib/ui/page/rename_conversation/view_model/rename_conversation_state.dart (1)
8-8
: Good implementation of sealed class patternConverting
RenameConversationState
to a sealed class maintains consistency with other state classes in the codebase and improves type safety. This change is part of the systematic approach to modernizing the codebase for Flutter SDK 3.29.0 and enhances the robustness of the state management system.lib/model/api/base/data_response.dart (1)
8-16
:❓ Verification inconclusive
Check downstream subclassing & Freezed version before committing to
sealed
Declaring these previously-open model classes as
sealed
will prevent any code outside the current library from subclassing them.
If any consumer package (or code in another Dart library of this repo) relied onclass Foo extends DataResponse
, that code will stop compiling after this PR.Additionally,
sealed
support in Freezed landed in v2.4.0. Ensurepubspec.yaml
hasfreezed: ^2.4.0 freezed_annotation: ^2.4.0(or newer) throughout the workspace, otherwise
build_runner
will fail.Please verify that
- No
extends DataResponse
/DataListResponse
/Meta
/PageInfo
exist outside this file.- All packages are bumped to Freezed ≥ 2.4.
You can run the quick search below to confirm point 1:
Also applies to: 19-27, 30-36, 39-45
🏁 Script executed:
#!/bin/bash # Look for external subclassing of the just-sealed classes. rg --no-heading -n $'extends\\s+(DataResponse|DataListResponse|Meta|PageInfo)\\b' | headLength of output: 87
Verify downstream subclassing & Freezed version
Sealing these model classes will prevent any code outside this library from extending them. Please ensure:
- No external
class … extends DataResponse
/DataListResponse
/Meta
/PageInfo
declarations exist in other Dart packages or libraries.- All
pubspec.yaml
files bumpfreezed
andfreezed_annotation
to at least^2.4.0
(or newer) so thatsealed
support is available.You can re-run these searches to double-check:
rg -n "extends\s+(DataResponse|DataListResponse|Meta|PageInfo)" . rg -n "freezed:" . rg -n "freezed_annotation:" .lib/ui_kit/paged_view/controller/common_paging_controller.dart (1)
29-41
: Verifystate.keys
and constructor args match infinite_scroll_pagination v5 API
getNextPageKey: (state) => (state.keys?.last ?? 0) + 1,
In v5 the public
PagingState
no longer exposes akeys
getter (it wasitemList
,nextPageKey
, etc.). Ifkeys
has been removed the above line will fail to compile.Likewise, the
PagingController
signature recently became:PagingController({ required PageRequestListener<PageKeyType, ItemType> fetchPage, required NextPageKeyGetter<ItemType, PageKeyType> getNextPageKey, PageKeyType firstPageKey, int invisibleItemsThreshold, })Confirm that:
state.keys
actually exists; otherwise derive the next key fromstate.nextPageKey
or a custom strategy.value:
can be nullable; some constructors require a non-null initialPagingState
.ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme (1)
62-63
: GPU validation mode can slow down runtime – ensure it is Debug-only
enableGPUValidationMode="1"
is excellent for catching Metal validation errors, but it incurs a noticeable performance penalty.
Because this setting sits inside the Debug-DevelopLaunchAction
, it is fine for local debugging, but please double-check that no Release / Profile schemes inherit it.super_lint/lib/src/base/rule_config.dart (1)
24-24
: Updated import statement to hide LintCode from analyzer packageThis change prevents importing the
LintCode
symbol from the analyzer package. This modification is necessary to avoid conflicts with custom LintCode implementations in the project, which is especially important when upgrading dependencies like the Flutter SDK.lib/model/firebase/firebase_user_data.dart (1)
11-11
: Upgraded class to sealed class for better type safetyConverting
FirebaseUserData
to a sealed class restricts subclassing and provides better type safety guarantees. This change aligns with the PR objective of adding sealed classes for functions generated byfreezed
and is part of the Flutter SDK 3.29.0 upgrade.lib/ui/page/all_users/view_model/all_users_state.dart (1)
9-9
: Updated to sealed class for improved type safetyConverting
AllUsersState
to a sealed class enhances type safety by restricting subclassing to only within this file. This aligns with modern Dart best practices and the PR's objective to update class declarations for the Flutter SDK 3.29.0 upgrade.lib/ui/page/login/login_page.dart (1)
141-143
: Replaced deprecated opacity method with newer APIThe code now uses
withValues(alpha:)
instead of the deprecatedwithOpacity()
method. This change improves code maintainability and follows the Flutter SDK 3.29.0 guidelines for handling opacity in color objects.lib/model/firebase/firebase_conversation_data.dart (1)
10-10
: Good update to use sealed classThe conversion to a sealed class aligns with the PR objective to add sealed classes for freezed-generated functions. This change restricts subclassing to this library only, providing better type safety and a more controlled inheritance hierarchy.
lib/model/firebase/firebase_reply_message_data.dart (1)
9-9
: Appropriate use of sealed classConverting to a sealed class improves type safety by restricting subclassing to this library. This change is consistent with the broader pattern applied to other model classes in the project.
lib/model/api/base/records_response.dart (1)
7-7
: Correct implementation of sealed class with genericsThe change to a sealed class for this generic response type is appropriate. It maintains the generic argument functionality while restricting inheritance, which helps prevent unintended extensions of this class outside the current library.
lib/model/base/load_more_output.dart (1)
8-8
: Proper conversion to sealed classThis change correctly implements the sealed class pattern for the LoadMoreOutput generic class. This is particularly important for classes like this that define core application data structures, as it prevents unexpected extensions that could lead to inconsistent behavior.
lib/ui/page/login/view_model/login_state.dart (1)
8-8
: Improved type safety with sealed classAdding the
sealed
keyword to this Freezed class enforces a more controlled inheritance hierarchy by restricting subclassing to the current library only. This change aligns with the PR's objective of adopting sealed classes for Freezed-generated functions throughout the codebase.lib/model/firebase/firebase_conversation_user_data.dart (1)
11-11
: Improved type safety with sealed classAdding the
sealed
keyword to this Freezed class restricts subclassing to the current library, enhancing type safety and enforcing a more controlled inheritance hierarchy. This change is consistent with the pattern applied across other Firebase and API model classes in the codebase.lib/model/base/paged_list.dart (1)
8-8
: Improved type safety with sealed classAdding the
sealed
keyword to thePagedList<T>
class restricts subclassing to the current library, enhancing type safety and aligning with the pattern applied to other model classes in the codebase.super_lint/pubspec.yaml (1)
10-17
: Updated dependency versionsThe dependency version updates align with the project's migration efforts mentioned in the PR objectives. These updates keep the linting tools in sync with the core project dependencies.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 11-11: trailing spaces
(trailing-spaces)
test/unit_test/ui/page/all_users/view_model/all_users_view_model_test.dart (1)
293-297
: Improved navigation verification approachThe change to use
isA<ChatRoute>().having()
matcher instead of direct equality checking is a good improvement. This makes the test more resilient by focusing on verifying the route type and specific properties (conversation argument) rather than exact instance equality.lib/ui/page/all_users/all_users_page.dart (1)
193-195
: Updated opacity implementationChanged from using
withOpacity()
towithValues(alpha:)
for setting button transparency, which aligns with the PR objective to remove deprecated opacity usage. This is a good modernization that explicitly sets the alpha channel value.super_lint/lib/src/index.dart (1)
12-12
: Hiding LintCode from analyzer error exportThis change prevents potential symbol conflicts by explicitly hiding the
LintCode
class from the analyzer package export, which is a good practice to avoid name collisions.lib/model/firebase/firebase_message_data.dart (1)
10-10
: Updated to sealed classConverting
FirebaseMessageData
to a sealed class aligns with the PR objective to use sealed classes for freezed-generated code. This change:
- Restricts inheritance to the current library
- Provides more type safety for your model hierarchy
- Enables better exhaustiveness checking in pattern matching
This is part of a broader pattern of modernizing the codebase to leverage newer Dart language features.
lib/ui/page/home/view_model/home_state.dart (1)
8-10
: Excellent upgrade to sealed class pattern!Converting
HomeState
to a sealed class aligns with modern Dart 3 practices and offers several benefits:
- Improved type safety through exhaustiveness checking in switch statements
- Restricted subclassing to the current library
- Better pattern matching support
- More explicit class inheritance control
The addition of the private unnamed constructor is the correct pattern when implementing sealed classes.
lib/ui/page/main/view_model/main_state.dart (1)
8-10
: Good implementation of sealed class pattern!Converting
MainState
to a sealed class follows the same pattern as other state classes in the codebase, providing consistent type safety improvements:
- Restricts inheritance to the current library
- Enables exhaustiveness checking when used with switch statements
- Follows recommended patterns for freezed classes in Dart 3
- The private constructor correctly prevents external instantiation
This change aligns perfectly with the PR objective of adding sealed classes to freezed-generated code.
lib/model/api/mock/mock_data.dart (2)
9-9
: Appropriate sealed class implementation for MockDataConverting
MockData
to a sealed class is consistent with the pattern applied across the codebase and enhances type safety.When combined with
@visibleForTesting
, this ensures that:
- The class is only visible in test code
- Subclassing is restricted to this library only
- Pattern matching is fully supported in test code
This change aligns with the PR objective of adding sealed classes to freezed-generated code.
21-21
: Consistent application of sealed class patternConverting
MockData2
to a sealed class maintains consistency with other data models in the codebase, ensuring uniform type safety and inheritance control.lib/ui/page/chat/view_model/chat_state.dart (1)
8-10
: Well-implemented sealed class for ChatState!Converting
ChatState
to a sealed class is particularly beneficial for chat functionality:
- Ensures exhaustive handling of all chat states in UI code
- Prevents unauthorized extension of the chat state hierarchy
- Provides better type safety for message and conversation handling
- Maintains consistency with other state classes in the codebase
The private constructor pattern is correctly implemented, matching the approach used in other state classes.
test/widget_test/ui/page/home/home_page_test.dart (2)
23-23
: API update for consistent user fetchingThe test has been updated to use the new unified
fetchUsers(isInitialLoad: true)
method instead of a separatefetchInitialUsers()
method, aligning with the refactored API in theHomeViewModel
class.
118-132
: Proper mocking of the paging controller's fetchPage methodThis change refactors the test to properly mock the new paging controller API. Instead of directly manipulating the controller's data, the test now mocks the
fetchPage
callback method, which is the correct approach for the upgraded pagination library (version 5).The updated pattern allows for better testing of the reactive pagination flow. The mock correctly generates test data with appropriate parameters to simulate page loading.
test/unit_test/ui/page/home/view_model/home_view_model_test.dart (1)
40-40
: Unified API for user fetchingAll instances of separate
fetchInitialUsers()
andfetchMoreUsers()
method calls have been consolidated to use the new unifiedfetchUsers(isInitialLoad: boolean)
method. This is a positive change that simplifies the API while maintaining the same functionality.This change makes the code more maintainable as the loading logic is centralized in a single method with different behaviors controlled by the
isInitialLoad
parameter. The tests continue to verify the same expected state transitions and behaviors.Also applies to: 80-80, 115-115, 152-152, 189-189, 227-227
lib/ui/page/setting/view_model/setting_state.dart (1)
8-10
: Enhanced type safety with sealed classThe
SettingState
class has been converted to asealed class
as part of the modernization efforts. This restricts inheritance to this library only and aligns with the migration to using sealed classes with Freezed, as mentioned in the PR objectives.The private constructor
const SettingState._();
is correctly added to support the sealed class pattern while preserving immutability, which is a best practice when using Freezed with sealed classes.android/build.gradle (3)
8-8
: Updated string literal syntaxChanged from single quotes to double quotes for consistency.
11-20
: Improved Android project configurationAdded an
afterEvaluate
block to automatically set the namespace property for Android projects if undefined. This is a good improvement that addresses potential issues with the new Android Gradle Plugin requirements for namespace definitions.This change is part of the broader Gradle build system upgrade mentioned in the PR objectives and ensures proper compatibility with newer Android SDK build tools.
21-23
: Separated project evaluation dependencySplit the subprojects configuration into two distinct blocks to better separate concerns. The explicit dependency on the
:app
project ensures proper evaluation order.This change improves the build script's organization and clarity while maintaining the same functionality.
lib/ui/page/home/view_model/home_view_model.dart (1)
19-23
: Clean refactoring to unify fetch methods.This change simplifies the API by consolidating separate fetch methods into a single parameterized method, which improves maintainability and reduces code duplication. The implementation correctly forwards the
isInitialLoad
parameter to the internal_getUsers
method.super_lint/lib/src/utils/lint_extensions.dart (1)
78-78
: Updated DartFormatter to explicitly use the latest language version.This change ensures that code formatting is done using the latest Dart language features, which is necessary for compatibility with the Flutter SDK 3.29.0 upgrade mentioned in the PR objectives.
lib/ui/page/home/home_page.dart (3)
26-32
: Good adoption of reactive pattern with fetchPage callback.The refactoring to use a single callback-based approach for data fetching simplifies the code and improves maintainability. This change aligns with the migration of the
infinite_scroll_pagination
package to version 5 mentioned in the PR objectives.
69-69
: Simplified refresh handling with controller's refresh method.Using
pagingController.refresh()
instead of directly calling a separate fetch method is more consistent with the new reactive pattern.
74-74
: Disabled animation transitions.Disabling animations may improve performance, especially on slower devices. Was this intended or is it a side effect of the migration?
lib/ui_kit/paged_view/common_paged_sliver_grid.dart (2)
73-89
: Well-structured extraction of pagedView function.Extracting this builder function improves code organization and readability. It cleanly separates the view construction from the state management.
91-98
: Good adoption of PagingListener for reactive updates.This change to a reactive pattern with
PagingListener
aligns with the migration to the newer version of the pagination library. It enables the widget to respond to state changes more efficiently.lib/ui_kit/paged_view/common_paged_grid_view.dart (2)
101-128
: Implementation of pagedView function is a good refactoring for infinite_scroll_pagination v5The new implementation correctly adapts to the API changes in version 5 of the infinite_scroll_pagination package. The function now takes explicit state and fetchNextPage parameters instead of relying on the controller directly, which follows a more reactive pattern.
130-138
: PagingListener implementation properly extracts paging stateThis is a good implementation of the reactive pattern with PagingListener. The widget now listens to state changes from the controller and rebuilds only when needed, passing the current state and fetch callback to the view builder. This approach improves separation of concerns and maintainability.
lib/ui_kit/paged_view/common_paged_sliver_list.dart (2)
71-98
: Well-structured pagedView function with conditional SeparatedList supportThe implementation correctly handles both regular and separated list views based on the presence of a separatorBuilder. The function properly passes the PagingState and fetchNextPage callback to the PagedSliverList components.
100-108
: Effective use of PagingListener consistent with other paged viewsThe PagingListener implementation matches the pattern used in CommonPagedGridView, ensuring consistency across the UI kit components. This maintains a unified approach to handling paging state changes.
android/settings.gradle (2)
1-17
: Modern Gradle configuration with pluginManagement blockThe refactoring to use the pluginManagement block is a good modernization of the Android build system. The proper extraction of the Flutter SDK path with an assertion ensures build reliability, and the explicit repository declarations follow best practices for Gradle 8.x.
19-25
: Centralized plugin version management with plugins DSLThe new plugins block correctly defines versions for all required plugins, which is a cleaner approach than the old buildscript configuration. The explicit version declarations (8.1.1 for Android Gradle Plugin, 1.9.22 for Kotlin, etc.) make dependency management more transparent.
lib/ui_kit/paged_view/common_paged_list_view.dart (3)
116-118
: Updated pagedView signature for new paging APIThe function signature has been properly updated to accept PagingState and fetchNextPage callback instead of directly using the controller, which aligns with the new API pattern in infinite_scroll_pagination v5.
121-122
: Consistent parameter updates across both list view implementationsThe parameter updates have been consistently applied to both the regular PagedListView and PagedListView.separated implementations, correctly using the state and fetchNextPage arguments.
Also applies to: 151-152
182-191
: PagingListener integration with LayoutBuilderThe integration of PagingListener with LayoutBuilder is well-implemented, maintaining access to the constraint dimensions while utilizing the new reactive paging pattern. This ensures the widget continues to work with features like snapping physics that depend on layout constraints.
pubspec.yaml (6)
5-5
: Good practice using a YAML anchor for Isar version.Using a YAML anchor (
&isar_version
) for the Isar version is a great practice for maintaining consistency across multiple dependencies. This aligns with the PR objective to migrate the Isar library to the community-maintained version.
11-14
: Dependency overrides ensure compatibility with Flutter SDK 3.29.0.These precise version overrides for analyzer, dart_style, freezed_annotation, and source_gen will ensure compatibility with the upgraded Flutter SDK mentioned in the PR objectives.
59-61
: Updated hooks_riverpod and infinite_scroll_pagination versions.The upgrade of hooks_riverpod to 2.6.1 and infinite_scroll_pagination to 5.0.0 aligns with the PR objective to migrate the infinite_scroll_pagination package to version 5.
96-98
: Consistent Isar generator configuration.The isar_generator is correctly configured to use the same version alias and custom hosted source as the main Isar package, ensuring consistency.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 96-96: trailing spaces
(trailing-spaces)
109-109
: Fixed typo in flutter_gen configuration.Corrected
line_length
property name (previously misspelled as "line_lenght"), which is a good cleanup.
63-68
:✅ Verification successful
Migration to community-maintained Isar implementation.
This change redirects Isar dependencies to use the community-maintained version hosted at https://pub.isar-community.dev/, addressing the PR objective to migrate from the abandoned version.
🏁 Script executed:
#!/bin/bash # Verify that the Isar community version is active and maintained curl -s "https://pub.isar-community.dev/api/packages/isar" | jq '.latest.version, .latest.published'Length of output: 113
✅ Migration to community-hosted Isar dependencies verified
The
pubspec.yaml
entries forisar
andisar_flutter_libs
correctly point tohttps://pub.isar-community.dev/
, and the community server is live—returning version 4.0.3 of the package. No further changes are needed.• pubspec.yaml (lines 63–68): dependencies now reference the community‐maintained registry.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 63-63: trailing spaces
(trailing-spaces)
test/unit_test/ui/page/main/view_model/main_view_model_test.dart (3)
148-157
: Improved route verification with more flexible matchers.This change improves test robustness by focusing on the route type and conversation property rather than exact route instance comparison. This approach is less brittle and more maintainable.
194-202
: Consistent use of flexible matchers across all route verification tests.The pattern of using
isA<ChatRoute>().having()
matchers is applied consistently across multiple test methods, which improves code consistency and readability.Also applies to: 227-235, 259-267
305-311
: Enhanced route verification with captured arguments.This approach captures the actual route argument and explicitly verifies that the conversation ID differs from the destination ID, which makes the test intention clearer and provides better diagnostics on failure.
android/app/build.gradle (6)
1-8
: Modernized Gradle plugin application with plugins DSL.Updated to the modern Gradle plugins DSL syntax, which is more concise and maintainable than the older
apply plugin
approach. This aligns with the PR objective to upgrade the Gradle build system.
11-13
: Updated Android configuration with modern syntax.The namespace declaration and updated compileSdk to 35 ensure compatibility with the latest Android requirements. Using the
=
assignment syntax is the modern Gradle Kotlin DSL style.
16-19
: Added desugaring support for modern Java features.Enabling core library desugaring with the appropriate dependency ensures that modern Java APIs are available even when targeting older Android versions. The explicit Java and Kotlin compatibility settings provide clarity and consistency.
Also applies to: 22-22, 106-108
26-31
: Updated default config with modern syntax and MultiDex support.The modern property assignment syntax and addition of MultiDex support ensure the app can handle a large number of methods, which is important as the project grows with more dependencies.
34-55
: Simplified signingConfigs with better property loading.Improved the local properties loading approach for signing configuration. The code is now more structured and easier to maintain.
111-113
: Streamlined Flutter configuration block.Simplified the flutter configuration to only include the essential source directory declaration, removing redundant configurations.
lib/model/other/initial_resource.dart (1)
8-8
: Appropriate update to use sealed class with freezedThe change from a regular class to a sealed class appropriately restricts subclassing to the current library, improving type safety and pattern matching capabilities. This aligns with modern Dart practices and the PR's objective to upgrade to Flutter SDK 3.29.0.
lib/model/api/server_error_detail.dart (1)
6-6
: Good modernization with sealed classConverting
ServerErrorDetail
to a sealed class follows the same pattern applied consistently across the codebase. This improves type safety and enforces a more controlled inheritance hierarchy, which is particularly valuable for error handling classes.lib/model/api/api_refresh_token_data.dart (1)
7-7
: Appropriate use of sealed class with JSON serializationThe addition of the
sealed
keyword properly restricts subclassing while maintaining compatibility with JSON serialization. This change is consistent with the codebase-wide pattern and represents good modernization practice for Dart classes using freezed.
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.
Pull Request Overview
This PR upgrades the Flutter SDK to version 3.29.0 and updates several dependencies along with build configurations.
- Upgraded the FirebaseFirestore iOS SDK tag in the Podfile from 11.4.0 to 11.8.0.
- Modernized Android and iOS build configurations as part of broader dependency and tooling updates.
Files not reviewed (19)
- android/app/build.gradle: Language not supported
- android/build.gradle: Language not supported
- android/gradle.properties: Language not supported
- android/gradle/wrapper/gradle-wrapper.properties: Language not supported
- android/settings.gradle: Language not supported
- ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme: Language not supported
- lib/model/api/api_refresh_token_data.dart: Language not supported
- lib/model/api/api_user_data.dart: Language not supported
- lib/model/api/base/data_response.dart: Language not supported
- lib/model/api/base/records_response.dart: Language not supported
- lib/model/api/base/results_response.dart: Language not supported
- lib/model/api/mock/mock_data.dart: Language not supported
- lib/model/api/server_error.dart: Language not supported
- lib/model/api/server_error_detail.dart: Language not supported
- lib/model/base/load_more_output.dart: Language not supported
- lib/model/base/paged_list.dart: Language not supported
- lib/model/firebase/firebase_conversation_data.dart: Language not supported
- lib/model/firebase/firebase_conversation_user_data.dart: Language not supported
- lib/model/firebase/firebase_message_data.dart: Language not supported
Tickets
#1
Checklist
Notes (optional)
freezed
infinite_scroll_pagination
to v5isar
trên pub.dev bị abandoned. Migrate sang https://github.com/isar-community/isarlistenSelf
của riverpod bị deprecated. Chỗ này cần tạo PBI mới để update.Screenshot (If UI changed)
Summary by CodeRabbit
Refactor
Style
Tests
Chores
Documentation
Other
sealed
modifier to many data and state classes, enhancing type safety and maintainability.