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.

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

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

Merged
merged 2 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1975,10 +1975,7 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/js_interop/js_typed_data.dart
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/key_map.g.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/keyboard_binding.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/mouse_cursor.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/navigation.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/navigation/history.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/navigation/js_url_strategy.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/navigation/url_strategy.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/noto_font.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/onscreen_logging.dart + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/picture.dart + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4639,10 +4636,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/js_interop/js_typed_data.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/key_map.g.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/keyboard_binding.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/mouse_cursor.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/navigation.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/navigation/history.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/navigation/js_url_strategy.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/navigation/url_strategy.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/noto_font.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/onscreen_logging.dart
FILE: ../../../flutter/lib/web_ui/lib/src/engine/picture.dart
Expand Down
2 changes: 0 additions & 2 deletions lib/web_ui/lib/src/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ export 'engine/key_map.g.dart';
export 'engine/keyboard_binding.dart';
export 'engine/mouse_cursor.dart';
export 'engine/navigation/history.dart';
export 'engine/navigation/js_url_strategy.dart';
export 'engine/navigation/url_strategy.dart';
export 'engine/noto_font.dart';
export 'engine/onscreen_logging.dart';
export 'engine/picture.dart';
Expand Down
25 changes: 0 additions & 25 deletions lib/web_ui/lib/src/engine/initialization.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'dart:js_interop';

import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
import 'package:web_test_fonts/web_test_fonts.dart';

/// The mode the app is running in.
Expand Down Expand Up @@ -133,10 +132,6 @@ Future<void> initializeEngineServices({
// Store `jsConfiguration` so user settings are available to the engine.
configuration.setUserConfiguration(jsConfiguration);

// Setup the hook that allows users to customize URL strategy before running
// the app.
_addUrlStrategyListener();

// Called by the Web runtime just before hot restarting the app.
//
// This extension cleans up resources that are registered with browser's
Expand Down Expand Up @@ -263,26 +258,6 @@ Future<void> _downloadAssetFonts() async {
}
}

void _addUrlStrategyListener() {
jsSetUrlStrategy = allowInterop((JsUrlStrategy? jsStrategy) {
if (jsStrategy == null) {
ui_web.urlStrategy = null;
} else {
// Because `JSStrategy` could be anything, we check for the
// `addPopStateListener` property and throw if it is missing.
if (!hasJsProperty(jsStrategy, 'addPopStateListener')) {
throw StateError(
'Unexpected JsUrlStrategy: $jsStrategy is missing '
'`addPopStateListener` property');
}
ui_web.urlStrategy = CustomUrlStrategy.fromJs(jsStrategy);
}
});
registerHotRestartListener(() {
jsSetUrlStrategy = null;
});
}

/// Whether to disable the font fallback system.
///
/// We need to disable font fallbacks for some framework tests because
Expand Down
7 changes: 0 additions & 7 deletions lib/web_ui/lib/src/engine/navigation.dart

This file was deleted.

87 changes: 0 additions & 87 deletions lib/web_ui/lib/src/engine/navigation/js_url_strategy.dart

This file was deleted.

48 changes: 0 additions & 48 deletions lib/web_ui/lib/src/engine/navigation/url_strategy.dart

This file was deleted.

2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/test_embedding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class TestHistoryEntry {
///
/// It keeps a list of history entries and event listeners in memory and
/// manipulates them in order to achieve the desired functionality.
class TestUrlStrategy extends ui_web.UrlStrategy {
class TestUrlStrategy implements ui_web.UrlStrategy {
/// Creates a instance of [TestUrlStrategy] with an empty string as the
/// path.
factory TestUrlStrategy() => TestUrlStrategy.fromEntry(const TestHistoryEntry(null, null, ''));
Expand Down
11 changes: 0 additions & 11 deletions lib/web_ui/lib/src/engine/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
import '../engine.dart' show DimensionsProvider, registerHotRestartListener, renderer;
import 'dom.dart';
import 'navigation/history.dart';
import 'navigation/js_url_strategy.dart';
import 'platform_dispatcher.dart';
import 'services.dart';
import 'util.dart';
Expand Down Expand Up @@ -324,16 +323,6 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow {
ui.Size? webOnlyDebugPhysicalSizeOverride;
}

typedef _JsSetUrlStrategy = void Function(JsUrlStrategy?);

/// A JavaScript hook to customize the URL strategy of a Flutter app.
//
// DO NOT CHANGE THE JS NAME, IT IS PUBLIC API AT THIS POINT.
//
// TODO(mdebbar): Add integration test https://github.com/flutter/flutter/issues/66852
@JS('_flutter_web_set_location_strategy')
external set jsSetUrlStrategy(_JsSetUrlStrategy? newJsSetUrlStrategy);

/// The Web implementation of [ui.SingletonFlutterWindow].
class EngineSingletonFlutterWindow extends EngineFlutterWindow {
EngineSingletonFlutterWindow(
Expand Down
8 changes: 2 additions & 6 deletions lib/web_ui/lib/ui_web/src/ui_web/navigation/url_strategy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ typedef PopStateListener = void Function(Object? state);
///
/// By default, the [HashUrlStrategy] subclass is used if the app doesn't
/// specify one.
abstract class UrlStrategy {
/// Abstract const constructor. This constructor enables subclasses to provide
/// const constructors so that they can be used in const expressions.
const UrlStrategy();

abstract interface class UrlStrategy {
/// Adds a listener to the `popstate` event and returns a function that, when
/// invoked, removes the listener.
ui.VoidCallback addPopStateListener(PopStateListener fn);
Expand Down Expand Up @@ -139,7 +135,7 @@ abstract class UrlStrategy {
/// // Somewhere before calling `runApp()` do:
/// setUrlStrategy(const HashUrlStrategy());
/// ```
class HashUrlStrategy extends UrlStrategy {
class HashUrlStrategy implements UrlStrategy {
/// Creates an instance of [HashUrlStrategy].
///
/// The [PlatformLocation] parameter is useful for testing to mock out browser
Expand Down
5 changes: 2 additions & 3 deletions lib/web_ui/test/engine/history_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import 'package:quiver/testing/async.dart';
import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
import 'package:ui/src/engine.dart' show window;
import 'package:ui/src/engine/dom.dart'
show DomEvent, createDomPopStateEvent;
import 'package:ui/src/engine/navigation.dart';
import 'package:ui/src/engine/dom.dart' show DomEvent, createDomPopStateEvent;
import 'package:ui/src/engine/navigation/history.dart';
import 'package:ui/src/engine/services.dart';
import 'package:ui/src/engine/test_embedding.dart';
import 'package:ui/ui_web/src/ui_web.dart';
Expand Down