Skip to content

[web] Update package:web to ">=0.5.1 <2.0.0" #7202

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 10 commits into from
Aug 1, 2024

Conversation

ditman
Copy link
Member

@ditman ditman commented Jul 23, 2024

This PR updates packages that depend on package:web to web: ">=0.5.1 <2.0.0".

Issues

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@ditman
Copy link
Member Author

ditman commented Jul 23, 2024

(First push to see how CI likes this, expecting a bunch of errors!)

@ditman
Copy link
Member Author

ditman commented Jul 23, 2024

Because google_identity_services_web >=0.3.1+1 depends on web ^0.5.1 and google_identity_services_web >=0.3.1 <0.3.1+1 depends on web ^0.5.0, google_identity_services_web >=0.3.1 requires web ^0.5.0.
So, because google_sign_in_web depends on both google_identity_services_web ^0.3.1 and web ^1.0.0, version solving failed.
Because google_maps_flutter_web depends on google_maps ^7.1.0 which depends on web ^0.5.0, web ^0.5.0 is required.
So, because google_maps_flutter_web depends on web ^1.0.0, version solving failed.

Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Rexios80
Copy link
Member

Rexios80 commented Jul 24, 2024

@ditman There is some cleanup to do that won't show up as build errors since a lot of the import statements are like this:

import 'package:web/web.dart' as web;

which will hide duplicate type errors

package:web 1.0.0 added a lot of types, functions, fields, etc that currently exist in custom extensions. For example in video_player_web:

import 'package:web/web.dart' as web;

/// Adds a "disablePictureInPicture" setter to [web.HTMLVideoElement]s.
extension NonStandardSettersOnVideoElement on web.HTMLVideoElement {
  // TODO(srujzs): This will be added in `package:web` 0.6.0. Remove this helper
  // once it's available.
  external set disablePictureInPicture(bool disabled);
}

/// Adds a "disableRemotePlayback" and "controlsList" setters to [web.HTMLMediaElement]s.
extension NonStandardSettersOnMediaElement on web.HTMLMediaElement {
  // TODO(srujzs): This will be added in `package:web` 0.6.0. Remove this helper
  // once it's available.
  external set disableRemotePlayback(bool disabled);
  external set controlsList(String? controlsList);
}

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jul 24, 2024

For any packages without any code changes (which is currently all of them, but I'm not sure how preliminary that is), consider expanding the range to include 1.x instead of requiring 1.x. According to pub there are about 150 packages depending on web, including a bunch of popular ones. The more packages that do a gradual transition allowing 0.5 or 1.x for a little while instead of going straight to requiring 1.x, the fewer people will have resolver errors that prevent them from using the latest versions of all of their packages as the transition works its way through the ecosystem.

Not a requirement by any means, but something to consider.

auto-submit bot pushed a commit that referenced this pull request Jul 24, 2024
Brackets the package:web dependency in gis_web to allow for `">=0.5.1 <2.0.0"`.

## Issues

This is needed so we can rebase and land:

* Part of: #7202
@ditman
Copy link
Member Author

ditman commented Jul 24, 2024

package:web 1.0.0 added a lot of types, functions, fields, etc that currently exist in custom extensions. For example in video_player_web:

@Rexios80 you're right, I'll eventually get to those as we fix the more core dependencies. There's only a few "package_web_tweaks" in the different packages which should be "easy to remove".

will hide duplicate type errors

@Rexios80 we have fairly strict warnings-as-errors in the build; I think those errors will eventually show up once the "all_packages" app is able to resolve to web:^1.0.0. Or do you mean those will fail at run-time or some time after publishing?

For any packages without any code changes (which is currently all of them, but I'm not sure how preliminary that is), consider expanding the range to include 1.x instead of requiring 1.x.

Good call @stuartmorgan, I'll try to expand the untouched packages to a range, as long as the code is compatible.

@Rexios80
Copy link
Member

we have fairly strict warnings-as-errors in the build; I think those errors will eventually show up once the "all_packages" app is able to resolve to web:^1.0.0. Or do you mean those will fail at run-time or some time after publishing?

I'm not sure they will show up as warnings/errors since technically nothing will be wrong. The extensions will just shadow the real types (if the web import is named) and fields (always). They should still build fine, there will just be superfluous code.

@ditman ditman force-pushed the migrate-to-web-1 branch from 87d121a to 04f8c10 Compare July 24, 2024 23:12
@ditman
Copy link
Member Author

ditman commented Jul 24, 2024

I'm not sure they will show up as warnings/errors since technically nothing will be wrong. The extensions will just shadow the real types (if the web import is named) and fields (always). They should still build fine, there will just be superfluous code.

@Rexios80 Ah yes, I think this is fine (for a while). IMO it's very nice that we can write this type of "bridge" code across major versions for a change! 😭

We can (and should) remove the package_web_tweaks files once we bump the dependency to ^1.0.0 only (and not the range). For example, we could use this PR to clean up the small tweaks that google_maps_flutter_web has to package:web, if we want to make it web:1.0.0 only.

As @stuartmorgan said, the packages that don't require actual code changes to go to web:^1.0.0, should support the >=0.5.1 <2.0.0 range until they must be moved to ^1.0.0 (because of us needing some feature from the package). It seems that the tweaks needed to support the web range are pretty small anyways! 🤞

@Rexios80
Copy link
Member

As @stuartmorgan said, the packages that don't require actual code changes to go to web:^1.0.0, should support the >=0.5.1 <2.0.0 range until they must be moved to ^1.0.0 (because of us needing some feature from the package). It seems that the tweaks needed to support the web range are pretty small anyways! 🤞

In the past I would be opposed to maintaining a version range on a dependency since it's really easy to accidentally break backwards compatibility. However pana now checks packages after running pub downgrade so this is no longer an issue!

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jul 25, 2024

In the past I would be opposed to maintaining a version range on a dependency since it's really easy to accidentally break backwards compatibility. However pana now checks packages after running pub downgrade so this is no longer an issue!

We've been doing that in our CI, as part of presubmit checks, for two years. The pana check was actually added because I recommended it based on our setup.

@Rexios80 Rexios80 mentioned this pull request Jul 26, 2024
11 tasks
@ditman ditman requested a review from Rexios80 August 1, 2024 01:59
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@ditman
Copy link
Member Author

ditman commented Aug 1, 2024

(This should trigger all tests?)

Copy link
Member

@Rexios80 Rexios80 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small things I noticed. Otherwise LGTM!

@@ -2,7 +2,7 @@ name: web_benchmarks
description: A benchmark harness for performance-testing Flutter apps in Chrome.
repository: https://github.com/flutter/packages/tree/main/packages/web_benchmarks
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+web_benchmarks%22
version: 2.0.0
version: 2.0.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is a code change in this package, should this be 2.1.0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, the code changes are so minimal that I think a patch version should be enough.

@Rexios80
Copy link
Member

Rexios80 commented Aug 1, 2024

@ditman Should we ask for a test exemption on Discord?

@ditman
Copy link
Member Author

ditman commented Aug 1, 2024

@ditman Should we ask for a test exemption on Discord?

@Rexios80 it's almost 8PM PT, so it's probably a little bit late (or too early) for Discord.

@stuartmorgan has test exemption powers and can also answer your question about the web_benchmarks version; I'm sure this'll be ready to autosubmit early tomorrow!

@stuartmorgan-g
Copy link
Contributor

test-exempt: code refactor with no semantic change

@kevmoo
Copy link
Contributor

kevmoo commented Aug 1, 2024

auto-submit?

@Rexios80
Copy link
Member

Rexios80 commented Aug 1, 2024

@kevmoo There are some small tweaks left

@ditman
Copy link
Member Author

ditman commented Aug 1, 2024

OK, I've addressed all of the comments. It seems that if CI is happy, we're all happy, so adding autosubmit to this PR (will keep an eye on flakes just in case)

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2024
@auto-submit auto-submit bot merged commit 1c4b286 into flutter:main Aug 1, 2024
76 checks passed
@ditman ditman deleted the migrate-to-web-1 branch August 1, 2024 22:12
@ditman
Copy link
Member Author

ditman commented Aug 1, 2024

This is now published, thanks!

Run overview:
  packages/packages/cross_file - published
  packages/packages/file_selector/file_selector_web - published
  packages/packages/google_identity_services_web - published
  packages/packages/google_maps_flutter/google_maps_flutter_web - published
  packages/packages/google_sign_in/google_sign_in_web - published
  packages/packages/image_picker/image_picker_for_web - published
  packages/packages/pointer_interceptor/pointer_interceptor_web - published
  packages/packages/shared_preferences/shared_preferences_web - published
  packages/packages/url_launcher/url_launcher_web - published
  packages/packages/video_player/video_player_web - published
  packages/packages/web_benchmarks - published
  packages/packages/webview_flutter/webview_flutter_web - published

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 2, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
flutter/packages@27896d1...cc9ff47

2024-08-02 [email protected] [tool] Add note about clang version used in CI to tool readme (flutter/packages#7273)
2024-08-02 [email protected] [ci] Re-enable google_maps_flutter_web integration tests. (flutter/packages#7269)
2024-08-01 [email protected] [web] Update package:web to ">=0.5.1 <2.0.0" (flutter/packages#7202)
2024-08-01 [email protected] [go_router] Documentation for StatefulShellRoute (flutter/packages#6308)
2024-08-01 [email protected] [google_maps_flutter] Switch Android examples to TLHC (flutter/packages#7282)
2024-08-01 [email protected] Manual roll Flutter from f817e51 to e2e4398 (5 revisions) (flutter/packages#7276)
2024-08-01 [email protected] [various] Clean up C++ formatting (flutter/packages#7272)
2024-08-01 [email protected] [shared_preferences] fix stringlist bug (flutter/packages#7268)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade all packages to package:web version 1.0.0
5 participants