Skip to content

[google_maps_flutter_web] Migrate to google_maps: 8.0.0 #7077

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 45 commits into from
Jul 31, 2024

Conversation

@Rexios80
Copy link
Member Author

Rexios80 commented Jul 8, 2024

Why are the integration tests skipped? Is it because other steps are failing?

@Rexios80 Rexios80 changed the title [google_maps_flutter_web] package:web migration [google_maps_flutter_web] Migrate to google_maps: 8.0.0 Jul 8, 2024
@stuartmorgan-g
Copy link
Contributor

Why are the integration tests skipped? Is it because other steps are failing?

Because they are currently disabled

@ditman FYI in case you aren't aware; maybe something you could look at while gardening? @bparrishMines That's assigned to you, are you actively working on it? We shouldn't leave whole tests suites disabled for extended periods if we can avoid it.

@bparrishMines
Copy link
Contributor

@stuartmorgan Yea im going to take a look into it now. I'll try to narrow down the specific test to skip, so we no longer skip all the integration tests.

@Rexios80
Copy link
Member Author

@stuartmorgan is there an issue to track running integration tests compiled to WASM in CI?

@@ -32,6 +32,12 @@ dev_dependencies:
flutter_test:
sdk: flutter

dependency_overrides:
Copy link
Contributor

Choose a reason for hiding this comment

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

you'll want to drop this override, I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely

@kevmoo
Copy link
Contributor

kevmoo commented Jul 10, 2024

We also likely DON'T want to publish this package depending on a pre-release package.

@Rexios80
Copy link
Member Author

We also likely DON'T want to publish this package depending on a pre-release package.

Also definitely the case

@Rexios80
Copy link
Member Author

Should we update the documentation with instructions on how to add that script locally for WASM compilation?

@ditman
Copy link
Member

ditman commented Jul 26, 2024

The overlays_test.dart also hangs for me when running with flutter_plugin_tools so I'm not sure how you got it to pass 🤔. Maybe I'm missing something?

@Rexios80 not sure! I'm running the tests in linux with your latest changes to the drive-examples tool and the latest changes in flutter/flutter so assertions work in wasm:

$ flutter --version
Flutter 3.24.0-1.0.pre.287 • channel master • [email protected]:ditman/flutter-flutter.git
Framework • revision 7b5462cc34 (21 hours ago) • 2024-07-25 15:22:05 -0700
Engine • revision d665bf82dc
Tools • Dart 3.6.0 (build 3.6.0-82.0.dev) • DevTools 2.37.1

I use flutter from git, so whenever I upgrade and expect changes in the tools I have a script to clear the fluter cache:

rm /work/flutter/flutter/bin/cache/flutter_tools.* ;
flutter --version ;

I can drive the example both with and without --wasm, and they pass in both cases!

In fact, if you can put it in an arbitrary location, the repo-root third-party directory would be ideal.

@stuartmorgan I'll try to move it to the existing third_party in the repo root, and maybe have a symlink to it from the test setup. The file needs to be served from fluter drive/flutter run; I'm not sure how the scripts will handle the symlink.

Should we update the documentation with instructions on how to add that script locally for WASM compilation?

@Rexios80 yes, we should mention that the file needs to be served from the same-domain or a CORP/COEP enabled CDN (which unpkg is not...)... Or find another CDN that does COEP/CORP (?)

@ditman
Copy link
Member

ditman commented Jul 26, 2024

Ended up swapping the CDN we use in the example/web/index.html to jsdelivr and that seems to support wasm, so no need to host the file in our repo!

dit@dit:example$ flutter drive -d web-server --web-port=7357 --browser-name=chrome --driver test_driver/integration_test.dart --target integration_test/marker_clustering_test.dart --wasm
Resolving dependencies... 
Downloading packages... 
  _fe_analyzer_shared 72.0.0 (73.0.0 available)
  analyzer 6.7.0 (6.8.0 available)
  collection 1.18.0 (1.19.0 available)
! google_maps_flutter_web 0.5.9 from path .. (overridden)
  http_parser 4.0.2 (4.1.0 available)
  js 0.6.7 (0.7.1 available)
  material_color_utilities 0.11.1 (0.12.0 available)
  shelf 1.4.1 (1.4.2 available)
Got dependencies!
7 packages have newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.
Launching integration_test/marker_clustering_test.dart on Web Server in debug mode...
Compiling integration_test/marker_clustering_test.dart for the Web...        33.9s
✓ Built build/web
integration_test/marker_clustering_test.dart is being served at http://localhost:7357
The web-server device requires the Dart Debug Chrome extension for debugging. Consider using the Chrome or Edge
devices for an improved development workflow.
All tests passed.
Application finished.

(Also updated the README of the package to point to the jsdelivr URL)

@kevmoo
Copy link
Contributor

kevmoo commented Jul 26, 2024

@ditman I guess there's no way to avoid touching hosted resources for these tests...so 🤷

@Rexios80
Copy link
Member Author

Rexios80 commented Jul 26, 2024

I'm running the tests in linux

I'm on macOS. Could that be the cause?

@ditman What do you think about the COEP error?

@ditman
Copy link
Member

ditman commented Jul 26, 2024

@ditman What do you think about the COEP error?

I've seen this issue now, thanks! I'm going to try and reconfigure the COEP/CORP headers that are sent back by the flutter tool to see if we can still get proper WasmGC but with a slightly more permissive credentialless header value.

(The error is unrelated to this PR, it's a "behavior" in the way that cross-origin isolation works with 3P resources :P)


PS: I've tested with this PR:

And the error seems to go away. I'm not an expert in Wasm, so we'll see what @eyebrowsoffire has to say about it!

@ditman ditman self-requested a review July 31, 2024 01:26
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

Just bumped everything and this is looking good; if @Rexios80 is happy with the end result, let's merge! LGTM!

@Rexios80
Copy link
Member Author

@ditman thanks for the help cleaning this up!

LGTM once checks pass

@kevmoo
Copy link
Contributor

kevmoo commented Jul 31, 2024

Can we mark this auto-submit?

Copy link
Contributor

@kevmoo kevmoo 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 Rexios80 added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 31, 2024
@auto-submit auto-submit bot merged commit b0b4469 into flutter:main Jul 31, 2024
76 checks passed
@Rexios80
Copy link
Member Author

@kevmoo Checks were being flakey but they finally passed so yes we can

@ditman
Copy link
Member

ditman commented Aug 1, 2024

Yeeeeah! Thanks for landing this! 🎉

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

2024-08-01 [email protected] [local_auth_darwin] macOS Support  (flutter/packages#6267)
2024-08-01 [email protected] Manual roll Flutter from 4d12197 to f817e51 (7 revisions) (flutter/packages#7270)
2024-08-01 [email protected] [ci] version_check_command now checks markdown of first CHANGELOG line. (flutter/packages#7266)
2024-08-01 [email protected] Manual roll Flutter from 031dc3d to 4d12197 (12 revisions) (flutter/packages#7259)
2024-07-31 [email protected] [in_app_purchase_storekit] convert TranslatorTests to swift (flutter/packages#7232)
2024-07-31 [email protected] [google_maps_flutter_web] Migrate to `google_maps: 8.0.0` (flutter/packages#7077)
2024-07-31 [email protected] [tool] Run pre_publish.dart before publish --dry-run (flutter/packages#7258)

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@46a712f...27896d1

2024-08-01 [email protected] [local_auth_darwin] macOS Support  (flutter/packages#6267)
2024-08-01 [email protected] Manual roll Flutter from 4d12197 to f817e51 (7 revisions) (flutter/packages#7270)
2024-08-01 [email protected] [ci] version_check_command now checks markdown of first CHANGELOG line. (flutter/packages#7266)
2024-08-01 [email protected] Manual roll Flutter from 031dc3d to 4d12197 (12 revisions) (flutter/packages#7259)
2024-07-31 [email protected] [in_app_purchase_storekit] convert TranslatorTests to swift (flutter/packages#7232)
2024-07-31 [email protected] [google_maps_flutter_web] Migrate to `google_maps: 8.0.0` (flutter/packages#7077)
2024-07-31 [email protected] [tool] Run pre_publish.dart before publish --dry-run (flutter/packages#7258)

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@46a712f...27896d1

2024-08-01 [email protected] [local_auth_darwin] macOS Support  (flutter/packages#6267)
2024-08-01 [email protected] Manual roll Flutter from 4d12197 to f817e51 (7 revisions) (flutter/packages#7270)
2024-08-01 [email protected] [ci] version_check_command now checks markdown of first CHANGELOG line. (flutter/packages#7266)
2024-08-01 [email protected] Manual roll Flutter from 031dc3d to 4d12197 (12 revisions) (flutter/packages#7259)
2024-07-31 [email protected] [in_app_purchase_storekit] convert TranslatorTests to swift (flutter/packages#7232)
2024-07-31 [email protected] [google_maps_flutter_web] Migrate to `google_maps: 8.0.0` (flutter/packages#7077)
2024-07-31 [email protected] [tool] Run pre_publish.dart before publish --dry-run (flutter/packages#7258)

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
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_maps_flutter platform-web
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[google_maps_flutter_web] Support WASM compilation
5 participants