Upgrade Flutter and packages #1278

rajveermalviya · 2025-01-13T20:49:24Z

Fixes: #942

And update Flutter's supporting libraries to match.

Fixes: zulip#942 And migrate to using non-nullable generic types. Changelog: https://pub.dev/packages/pigeon/changelog#2272

Upgrade carried using by following commands: flutter pub upgrade --major-versions firebase_messaging firebase_core pod update --project-directory=ios/ pod update --project-directory=macos/ And adapted the test bindings to use the new `providesAppNotificationSettings` flag. Changelogs: https://pub.dev/packages/firebase_core/changelog#3100 https://pub.dev/packages/firebase_messaging/changelog#1520

…ter_lints This is the result of `tools/upgrade pub-major`. Changelogs: https://pub.dev/packages/device_info_plus/changelog#1120 https://pub.dev/packages/mime/changelog#200 https://pub.dev/packages/flutter_lints/changelog#500

PIG208

Thanks for the PR! I checked the changelogs from the upgrades. None of the breaking changes packages from the tools/upgrade pub-major upgrade affects us. The app also works fine on my Android device. It would be great to have it tested on someone's iOS device too.

Left some comments, but no action is required for moving this PR forward.

PIG208 · 2025-01-16T17:53:30Z

test/model/binding.dart

@@ -417,6 +418,7 @@ class FakeFirebaseMessaging extends Fake implements FirebaseMessaging {
    bool criticalAlert = false,
    bool provisional = false,
    bool sound = true,
+    bool providesAppNotificationSettings = false,


Not directly related, but I noticed that the documentation https://firebase.flutter.dev/docs/messaging/permissions/#permission-settings we linked at NotificationService._requestPermission does not seem up-to-date with the actual requestPermission method.

PIG208 · 2025-01-16T18:47:33Z

test/model/binding.dart

+  //   so return the default value.
+  @override
+  // ignore: non_constant_identifier_names
+  final String pigeonVar_messageChannelSuffix = '';


I wonder if dartHostTestHandler helps, but it is also subject to removal (flutter/flutter#159886). Mocking HostApi this way seems fine to me. Maybe these pigeonVar prefixed variables should be private in the generated code.

gnprice

Thanks @rajveermalviya for taking care of this, and @PIG208 for the previous review and the manual testing!

Generally this all looks good — one request below to split up a commit. The other comments are all just me reading the changelogs, the results of which are all that there's nothing more we need to do. I can add that information to the commit messages before merge.

The app also works fine on my Android device. It would be great to have it tested on someone's iOS device too.

For this set of upgrades I'm content with that testing you did, and not doing additional manual testing on iOS.

The main area where I'd be concerned about platform-specific issues that call for manual testing on both platforms is notifications. In this upgrade, the changelogs for the notification-related (Firebase-related) upgrades look quite benign.

gnprice · 2025-01-16T19:46:51Z

lib/notifications/display.dart

-    final List<StoredNotificationSound?> storedSounds;
+    final List<StoredNotificationSound> storedSounds;


deps: Upgrade pigeon to 22.7.2 Fixes: #942 And migrate to using non-nullable generic types.

Can the upgrade and the switch to non-nullable types be separated into two commits? I'd prefer to track which changes were required by the upgrade and which were our own choice.

gnprice · 2025-01-16T19:57:22Z

ios/Podfile.lock

-  - Firebase/CoreOnly (11.4.0):
-    - FirebaseCore (= 11.4.0)
-  - Firebase/Messaging (11.4.0):
+  - Firebase/CoreOnly (11.6.0):
+    - FirebaseCore (~> 11.6.0)
+  - Firebase/Messaging (11.6.0):


Here's the corresponding release notes:
https://firebase.google.com/support/release-notes/ios

from 11.4.0 up to 11.6.0. Most are in specific libraries we don't use, though, like Analytics or Vertex AI.

It looks like for the components we actually pull in, there's just a couple of build-time fixes in 11.4.1 and 11.4.2, then some error logging in FCM in 11.5.0. All quite innocuous, then.

gnprice · 2025-01-16T19:58:35Z

pubspec.lock

-    version: "3.9.0"
+    version: "3.10.0"


Looks like one change at https://pub.dev/packages/firebase_core/changelog : bumps the version of the Firebase iOS SDK.

Which is probably fine but opens the question of what changed there. (OK, found that changelog and posted another comment just now with the results.)

gnprice · 2025-01-16T19:59:26Z

pubspec.lock

-    version: "15.1.6"
+    version: "15.2.0"


Cool, changelog all looks innocuous.

gnprice · 2025-01-16T23:31:46Z

pubspec.yaml

-  device_info_plus: ^10.0.1
+  device_info_plus: ^11.2.0


Looks like the one "breaking" change in this changelog is:

BREAKING FIX(device_info_plus): fixed webasm compliance (#3254). (e35e2123)

so I'll assume that's web-only and has no effect on us.

gnprice · 2025-01-16T23:33:02Z

pubspec.yaml

-  mime: ^1.0.5
+  mime: ^2.0.0


The breaking change here is in extensionFromMime.

Looks like we don't use that method:

$ git grep extensionFromMime $

so doesn't affect us.

gnprice · 2025-01-16T23:40:14Z

pubspec.yaml

-  flutter_lints: ^4.0.0
+  flutter_lints: ^5.0.0


By nature if this doesn't cause CI to fail, it can't have broken anything.

There is one interesting change:

Removes the following lints (see https://github.com/dart-lang/lints/issues 205):

prefer_const_constructors

prefer_const_declarations

prefer_const_literals_to_create_immutables

So that ends the regular nagging from the analyzer to add const everywhere. As discussed in the thread, it's not clear that the performance benefit of it is/was material.

chrisbobbe · 2025-01-17T22:36:30Z

pubspec.yaml

-  sdk: '>=3.7.0-267.0.dev <4.0.0'
-  flutter: '>=3.28.0-2.0.pre.38575'  # 65ff060283e19423e9538c18c24e44495b70aeff
+  sdk: '>=3.7.0-312.0.dev <4.0.0'
+  flutter: '>=3.28.0-2.0.pre.38699'  # d14140f85439c517c98b0c40f8eaf942fcb46c74


Huh, I guess we still can't do #1204 then. 🤷‍♂️

rajveermalviya force-pushed the flutter-upgrade branch from 5b54b63 to 829a166 Compare January 13, 2025 21:17

rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Jan 13, 2025

PIG208 self-assigned this Jan 16, 2025

rajveermalviya added 5 commits January 16, 2025 22:31

deps: Upgrade Flutter to 3.28.0-2.0.pre.38699

117f863

And update Flutter's supporting libraries to match.

deps: Upgrade pigeon to 22.7.2

af4fc71

Fixes: zulip#942 And migrate to using non-nullable generic types. Changelog: https://pub.dev/packages/pigeon/changelog#2272

deps: Upgrade packages within constraints (tools/upgrade pub)

85cc8ee

rajveermalviya force-pushed the flutter-upgrade branch from 829a166 to 107c6c7 Compare January 16, 2025 17:04

PIG208 reviewed Jan 16, 2025

View reviewed changes

PIG208 added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Jan 16, 2025

PIG208 requested a review from gnprice January 16, 2025 18:50

PIG208 assigned gnprice and unassigned PIG208 Jan 16, 2025

gnprice reviewed Jan 16, 2025

View reviewed changes

chrisbobbe reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Flutter and packages #1278

Upgrade Flutter and packages #1278

rajveermalviya commented Jan 13, 2025

PIG208 left a comment •

edited

Loading

PIG208 Jan 16, 2025

PIG208 Jan 16, 2025

gnprice left a comment

gnprice Jan 16, 2025

gnprice Jan 16, 2025

gnprice Jan 16, 2025

gnprice Jan 16, 2025

gnprice Jan 16, 2025

gnprice Jan 16, 2025

gnprice Jan 16, 2025

chrisbobbe Jan 17, 2025 •

edited

Loading

		final List<StoredNotificationSound?> storedSounds;
		final List<StoredNotificationSound> storedSounds;

Upgrade Flutter and packages #1278

Are you sure you want to change the base?

Upgrade Flutter and packages #1278

Conversation

rajveermalviya commented Jan 13, 2025

PIG208 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisbobbe Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

PIG208 left a comment •

edited

Loading

chrisbobbe Jan 17, 2025 •

edited

Loading