[go_router] migrates to material_ui and cupertino_ui - #12504
Conversation
![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 updates the minimum supported SDK version to Flutter 3.44/Dart 3.12, simplifies several constructor initializations using initializing formals, and replaces imports of the official Flutter SDK Material library with a third-party material_ui package across the codebase, examples, and tests. Feedback indicates that migrating to the third-party material_ui package is incorrect for a core Flutter package and its examples, and these changes should be reverted to use the official Material library.
| logging: ^1.0.0 | ||
| material_ui: ^1.0.0 | ||
| meta: ^1.7.0 |
There was a problem hiding this comment.
Adding material_ui as a dependency is incorrect. go_router is a core Flutter package and must rely on the official Material library (package:flutter/material.dart) provided by the Flutter SDK rather than an external third-party package.
logging: ^1.0.0\n meta: ^1.7.0| // ignore_for_file: diagnostic_describe_all_properties | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:material_ui/material_ui.dart'; |
There was a problem hiding this comment.
Replacing package:flutter/material.dart with package:material_ui/material_ui.dart is incorrect. Please revert this import to use the official Flutter SDK Material library.
| import 'package:material_ui/material_ui.dart'; | |
| import 'package:flutter/material.dart'; |
There was a problem hiding this comment.
lol. Get with the times. :)
| logging: ^1.0.0 | ||
| material_ui: ^1.0.0 | ||
| shared_preferences: ^2.0.11 |
There was a problem hiding this comment.
Adding material_ui as a dependency in the example is incorrect. The example should use the official Flutter SDK Material library.
logging: ^1.0.0\n shared_preferences: ^2.0.11
| - [State restoration](https://pub.dev/documentation/go_router/latest/topics/State%20restoration-topic.html) | ||
|
|
||
| ## Migration Guides | ||
| - [Migrating to 18.0.0](https://flutter.dev/go/go-router-v18-breaking-changes). |
There was a problem hiding this comment.
github-actions
Bot
added
p: go_router
triage-framework
| @@ -4,8 +4,8 @@ | |||
|
|
|||
| import 'package:collection/collection.dart'; | |||
| import 'package:flutter/cupertino.dart'; | |||
There was a problem hiding this comment.
This PR doesn't complete the job. There are two parts to the split - one is material_ui, and the other is cupertino_ui, and these need to be migrated as well.
|
@chunhtai unfortunately I suspect that this migration (great start) is incomplete. It misses completely all the |
|
Yes, this should adopt cupertino_ui as well. |
|
oh i totally missed that. thanks for reminding |
chunhtai
changed the title
|
ready for another look |
| sdk: ^3.10.0 | ||
| flutter: ">=3.38.0" | ||
| sdk: ^3.12.0 | ||
| flutter: ">=3.44.0" |
There was a problem hiding this comment.
I wonder about this, @chunhtai . Should it be 3.44.0?
I mean the material_ui/cupertino_ui split only came in 3.47.0
Maybe it's backward compatible? I have no idea.
There was a problem hiding this comment.
I see the cupertino_ui says it's compatible from 3.44.0 - so perhaps this is good to go...
https://github.com/flutter/packages/blob/main/packages/cupertino_ui/pubspec.yaml
There was a problem hiding this comment.
I was following this comment in this pr #12467 (comment)
There was a problem hiding this comment.
oh this is to match the lower bound for material_ui
3 participants
fixes flutter/flutter#191133
fixes flutter/flutter#191132
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2