Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[web] Remove the JS API for url strategy #42134

Merged
merged 2 commits into from May 31, 2023
Merged

[web] Remove the JS API for url strategy #42134

merged 2 commits into from May 31, 2023

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented May 18, 2023

Finally, we can remove this JS global function for customizing the url strategy.

Why I think we don't need to go through an official deprecation process:

  1. It was initially made for internal use in Google3, and right now there are no references to it.
  2. There's no public documentation of this JS function.
  3. External users customize their url strategy through flutter_web_plugins which has been migrated already.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label May 18, 2023
eyebrowsoffire
eyebrowsoffire approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

LGTM!

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label May 31, 2023
@auto-submit auto-submit bot merged commit c1ef0d9 into flutter:main May 31, 2023
29 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 31, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 31, 2023
@engine-flutter-autoroll
c1ef0d995 [web] Remove the JS API for url strategy (flutter/engine#42134
0d9ba9a 
)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 1, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 1, 2023
@engine-flutter-autoroll
c1ef0d995 [web] Remove the JS API for url strategy (flutter/engine#42134
f3030dd 
)
jonahwilliams added a commit that referenced this pull request Jun 1, 2023
@jonahwilliams
Revert "[web] Remove the JS API for url strategy (#42134)"
b017b9b 
This reverts commit c1ef0d9.
@jonahwilliams jonahwilliams mentioned this pull request Jun 1, 2023
Merged
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 1, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 1, 2023
@engine-flutter-autoroll
c1ef0d995 [web] Remove the JS API for url strategy (flutter/engine#42134
1d253ce 
)
auto-submit bot pushed a commit that referenced this pull request Jun 1, 2023
@jonahwilliams
Revert "[web] Remove the JS API for url strategy" (#42468)
6db2f3e 
Reverts #42134

This is blocking the engine into framework roller:

See: https://cirrus-ci.com/task/5610586755563520

```
Analyzing 3 items...                                            
  error � The class 'UrlStrategy' can't be extended outside of its library because it's an interface class � dev/integration_tests/web_e2e_tests/test_driver/url_strategy_integration.dart:48:31 � invalid_use_of_type_outside_library
1 issue found. (ran in 321.8s)
  �  �  
  ```
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 1, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 1, 2023
@engine-flutter-autoroll
c1ef0d995 [web] Remove the JS API for url strategy (flutter/engine#42134
360eaf0 
)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 1, 2023
@engine-flutter-autoroll
Roll Flutter Engine from f9bc342c84e2 to 6db2f3e9857c (4 revisions) (#…
3df72ed 
…128009)

flutter/engine@f9bc342...6db2f3e

2023-06-01 jonahwilliams@google.com Revert "[web] Remove the JS API for url strategy" (flutter/engine#42468)
2023-06-01 skia-flutter-autoroll@skia.org Roll Dart SDK from e0caea366989 to 78d09b5874fa (4 revisions) (flutter/engine#42465)
2023-06-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from lSKDoGVypQfTMYUZe... to htio0wC3kDb9tB1Wd... (flutter/engine#42463)
2023-05-31 mdebbar@google.com [web] Remove the JS API for url strategy (flutter/engine#42134)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from lSKDoGVypQfT to htio0wC3kDb9

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jonahwilliams@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
mdebbar added a commit to mdebbar/engine that referenced this pull request Jun 1, 2023
@mdebbar
Reland "[web] Remove the JS API for url strategy (flutter#42134)"
f5c575b 
This reverts commit 6db2f3e.
@mdebbar mdebbar mentioned this pull request Jun 1, 2023
Merged
auto-submit bot pushed a commit that referenced this pull request Jun 1, 2023
@mdebbar
Reland "[web] Remove the JS API for url strategy (#42134)" (#42486)
96685f5 
Initially landed in #42134
Then reverted in #42468

It failed because we changed `UrlStrategy` to an `interface` which prevents "extending".

The only change in the reland is the removal of the `interface` keyword.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eyebrowsoffire eyebrowsoffire eyebrowsoffire approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
2 participants
@mdebbar @eyebrowsoffire