Skip to content

Remove unnecessary imports of dart:ui #447

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

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

sirpengi
Copy link
Contributor

@sirpengi sirpengi commented Dec 8, 2023

Upstream Flutter (in flutter/flutter@c6741614041 ) changed flutter/material.dart to additionally export many types from dart:ui. This caused a lot of our imports to trigger an analysis warning of unnecessary_import.

Please make sure you update your Flutter SDK after pulling in these changes!

@chrisbobbe
Copy link
Collaborator

Thanks!

If you're on a Flutter version that doesn't have that commit, won't the imports still be necessary? I think we should remove the imports alongside a bump of our lower bound on the Flutter version. For how to bump that lower bound, see: https://github.com/zulip/zulip-flutter?tab=readme-ov-file#upgrading-flutter , and for an example, see #387.

I guess it would make sense to do the min-version bump in the same commit as we remove the imports. Removing them becomes possible at the new Flutter version, but it also becomes necessary at the same time, because of the analysis rules we use. For the commit message, I would probably copy the form of a34b9e5, but with a brief note about removing the imports.

@sirpengi sirpengi force-pushed the bugfix-dartui-unnecessary-imports branch from a9d43d5 to c06855e Compare December 9, 2023 10:46
@sirpengi
Copy link
Contributor Author

sirpengi commented Dec 9, 2023

@chrisbobbe thanks for the comment! I've bumped the version and reframed the commit message.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Dec 18, 2023

LGTM, thanks! Greg, over to you.

And update Flutter's supporting libraries to match.

Also remove now-unnecessary imports of `dart:ui`. Upstream Flutter
(in flutter/flutter@c6741614041 ) changed `flutter/painting.dart`
and others to additionally export several more types from
`dart:ui`. This caused a lot of our imports to trigger an
analysis warning of `unnecessary_import`.
@gnprice
Copy link
Member

gnprice commented Dec 20, 2023

Thanks to you both! Looks good; merging, with small tweaks to the commit message.

(In particular it's not about flutter/material but about a lower-level library, flutter/painting. In that upstream commit, the key change is in packages/flutter/lib/src/painting/basic_types.dart. If it were about flutter/material then our lib/widgets/text.dart wouldn't be affected, because the latter imports only flutter/widgets which doesn't depend on flutter/material.)

@gnprice gnprice force-pushed the bugfix-dartui-unnecessary-imports branch from 0775369 to baecf80 Compare December 20, 2023 22:29
@gnprice gnprice merged commit baecf80 into zulip:main Dec 20, 2023
@sirpengi sirpengi deleted the bugfix-dartui-unnecessary-imports branch January 23, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants