Skip to content

fix: add support for non-root base-href #75

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

fix: add support for non-root base-href #75

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8892812
fix: add support for non-root base-href
holzgeist Sep 18, 2024
31526ae
Moved no_sleep.js to lib/assets.
diegotori Dec 3, 2024
606ade6
Added web alias to all package:web calls in import_js_library.dart.
diegotori Dec 3, 2024
b4114a9
Added more simplified library URL method in import_js_library.dart.
diegotori Dec 3, 2024
e948054
WakelockPlusWebPlugin now lazily loads no_sleep.js on demand as oppos…
diegotori Dec 3, 2024
9e1fd5b
Got the WakelockPlusWebPlugin tests passing.
diegotori Dec 3, 2024
39f6598
Fixed example app web-related depreactions.
diegotori Dec 3, 2024
f2542bd
Added straggler from previous commit.
diegotori Dec 3, 2024
b8d0b58
Added correct script tag error handling by checking if the target is …
diegotori Dec 3, 2024
bd3e7a7
Upgraded flutter_lints to latest version.
diegotori Dec 3, 2024
4900aaf
Revamped the no_sleep.js script to use a Promise-based Completer so t…
diegotori Dec 3, 2024
ac4e069
Fixed a dart lint issue due to HTML in the documentation of _importJS…
diegotori Dec 3, 2024
a7ddb22
Added minor comments to no_sleep.js.
diegotori Dec 3, 2024
897dca4
Merge branch 'main' into fix-support-non-root-base-href
diegotori Dec 21, 2024
f5d5bc4
Fixed a pubspec straggler from the previous commit.
diegotori Dec 21, 2024
8f38c22
Update wakelock_plus/lib/assets/no_sleep.js due to typo
diegotori Jan 8, 2025
bb3d4ca
chore: apply suggestions
holzgeist Feb 20, 2025
be5d5f5
Merge branch 'main' into fix-support-non-root-base-href
diegotori Mar 26, 2025
bb5d60d
Moved no_sleep.js copyright notice to the top of the file.
diegotori Mar 26, 2025
12cd725
fix: don't perform any work after completing future
holzgeist Apr 17, 2025
8f1ed7a
Merge branch 'main' into fix-support-non-root-base-href
diegotori Apr 17, 2025
3886996
Merge branch 'main' into fix-support-non-root-base-href
diegotori Apr 17, 2025
27f6d07
Merge branch 'main' into fix-support-non-root-base-href
diegotori Apr 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions wakelock_plus/example/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@
/build/

# Web related
lib/generated_plugin_registrant.dart

# Symbolication related
app.*.symbols

# Obfuscation related
app.*.map.json
!/ios/Podfile
!/ios/Podfile
23 changes: 1 addition & 22 deletions wakelock_plus/example/web/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,8 @@

<title>example</title>
<link rel="manifest" href="manifest.json">

<script>
// The value below is injected by flutter build, do not touch.
var serviceWorkerVersion = null;
</script>
<!-- This script adds the flutter initialization JS code -->
<script src="flutter.js" defer></script>
</head>
<body>
<script>
window.addEventListener('load', function(ev) {
// Download main.dart.js
_flutter.loader.loadEntrypoint({
serviceWorker: {
serviceWorkerVersion: serviceWorkerVersion,
},
onEntrypointLoaded: function(engineInitializer) {
engineInitializer.initializeEngine().then(function(appRunner) {
appRunner.runApp();
});
}
});
});
</script>
<script src="flutter_bootstrap.js" async></script>
</body>
</html>
0 wakelock_plus/assets/no_sleep.js → wakelock_plus/lib/assets/no_sleep.js
View file
Open in desktop
File renamed without changes.
32 changes: 21 additions & 11 deletions wakelock_plus/lib/src/wakelock_plus_web_plugin.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import 'dart:async';
import 'dart:js_interop';

import 'package:flutter_web_plugins/flutter_web_plugins.dart';
import 'package:wakelock_plus_platform_interface/wakelock_plus_platform_interface.dart';
import 'package:wakelock_plus/src/web_impl/import_js_library.dart';
import 'package:wakelock_plus/src/web_impl/js_wakelock.dart'
as wakelock_plus_web;
import 'package:wakelock_plus_platform_interface/wakelock_plus_platform_interface.dart';

/// The web implementation of the [WakelockPlatformInterface].
///
Expand All @@ -14,30 +14,40 @@ class WakelockPlusWebPlugin extends WakelockPlusPlatformInterface {
/// Registers [WakelockPlusWebPlugin] as the default instance of the
/// [WakelockPlatformInterface].
static void registerWith(Registrar registrar) {
// Import a version of `NoSleep.js` that was adjusted for the wakelock
// plugin.
_jsLoaded = importJsLibrary(
url: 'assets/no_sleep.js', flutterPluginName: 'wakelock_plus');

WakelockPlusPlatformInterface.instance = WakelockPlusWebPlugin();
}

// The future that resolves when the JS library is loaded.
static late Future<void> _jsLoaded;
// The future that signals when the JS is loaded.
// This needs to be `await`ed before accessing any methods of the
// JS-interop layer.
late Future<void> _jsLoaded;
bool _jsLibraryLoaded = false;

//
// Lazily imports the JS library once, then awaits to ensure that
// it's loaded into the DOM.
//
Future<void> _ensureJsLoaded() async {
if (!_jsLibraryLoaded) {
_jsLoaded = importJsLibrary(
url: 'assets/no_sleep.js', flutterPluginName: 'wakelock_plus');
_jsLibraryLoaded = true;
}
await _jsLoaded;
}

@override
Future<void> toggle({required bool enable}) async {
// Make sure the JS library is loaded before calling it.
await _jsLoaded;
await _ensureJsLoaded();

wakelock_plus_web.toggle(enable);
}

@override
Future<bool> get enabled async {
// Make sure the JS library is loaded before calling it.
await _jsLoaded;

await _ensureJsLoaded();
final completer = Completer<bool>();

wakelock_plus_web.enabled().toDart.then(
Expand Down
39 changes: 28 additions & 11 deletions wakelock_plus/lib/src/web_impl/import_js_library.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:js_interop';
import 'dart:ui_web' as ui_web;

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

/// This is an implementation of the `import_js_library` plugin that is used
/// until that plugin is migrated to null safety.
Expand All @@ -18,19 +19,23 @@ Future<void> importJsLibrary(
}

String _libraryUrl(String url, String pluginName) {
// Added suggested changes as per
// https://github.com/fluttercommunity/wakelock_plus/issues/19#issuecomment-2301963609
if (url.startsWith('./')) {
url = url.replaceFirst('./', '');
return './assets/packages/$pluginName/$url';
}

if (url.startsWith('assets/')) {
return './assets/packages/$pluginName/$url';
} else {
return url;
return ui_web.assetManager.getAssetUrl(
'packages/$pluginName/$url',
);
}

return url;
}

HTMLScriptElement _createScriptTag(String library) {
final script = document.createElement('script') as HTMLScriptElement
web.HTMLScriptElement _createScriptTag(String library) {
final script = web.document.createElement('script') as web.HTMLScriptElement
..type = 'text/javascript'
..charset = 'utf-8'
..async = true
Expand All @@ -42,32 +47,44 @@ HTMLScriptElement _createScriptTag(String library) {
/// Future that resolves when all load.
Future<void> _importJSLibraries(List<String> libraries) {
final loading = <Future<void>>[];
final head = document.head;
final head = web.document.head;

for (final library in libraries) {
if (!_isImported(library)) {
final scriptTag = _createScriptTag(library);
head!.appendChild(scriptTag);
loading.add(scriptTag.onLoad.first);
scriptTag.onError.listen((event) {
final scriptElement = event.srcElement is web.HTMLScriptElement
? event.srcElement as web.HTMLScriptElement
: null;
if (scriptElement != null) {
loading.add(
Future.error(
Exception('Error loading: ${scriptElement.src}'),
),
);
}
});
}
}

return Future.wait(loading);
}

bool _isImported(String url) {
final head = document.head!;
final head = web.document.head!;
return _isLoaded(head, url);
}

bool _isLoaded(HTMLHeadElement head, String url) {
bool _isLoaded(web.HTMLHeadElement head, String url) {
if (url.startsWith('./')) {
url = url.replaceFirst('./', '');
}
for (int i = 0; i < head.children.length; i++) {
final element = head.children.item(i)!;
if (element.instanceOfString('HTMLScriptElement')) {
if ((element as HTMLScriptElement).src.endsWith(url)) {
if ((element as web.HTMLScriptElement).src.endsWith(url)) {
Copy link

@ditman ditman Jan 8, 2025

Choose a reason for hiding this comment

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

I know this change is unrelated, but consider adding an id attribute to your script element, that way this method can be simplified to:

return head.querySelector('#$idForScriptElement') != null;

You may need to keep a small map of urls to each of its idForScriptElement String (probably it'll only contain a single element?)

Copy link
Author

@holzgeist holzgeist Feb 20, 2025

Choose a reason for hiding this comment

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

done. I hope the locking part works out as planned. These things are really hard to test. The happy path works though

return true;
}
}
Expand Down
2 changes: 1 addition & 1 deletion wakelock_plus/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,4 @@ flutter:
# https://flutter.dev/custom-fonts/#from-packages

assets:
- assets/no_sleep.js
- packages/wakelock_plus/assets/no_sleep.js
13 changes: 12 additions & 1 deletion wakelock_plus/test/wakelock_plus_web_plugin_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ import 'package:wakelock_plus/src/wakelock_plus_web_plugin.dart';
import 'package:wakelock_plus/wakelock_plus.dart';
import 'package:wakelock_plus_platform_interface/wakelock_plus_platform_interface.dart';

///
/// Run these tests with:
/// flutter run -d chrome test/wakelock_plus_web_plugin_test.dart
Copy link

@ditman ditman Jan 8, 2025

Choose a reason for hiding this comment

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

This is better than nothing, but super non-standard! If you want to run tests like this, I'd suggest you try to migrate them to flutter drive, like what we did in flutter/packages, for example:

https://github.com/flutter/packages/tree/main/packages/video_player/video_player_web/example <- this "example" actually contains the integration_test and all that jazz. The README.md should have some links to the docs on how to run those.

Copy link
Collaborator

@diegotori diegotori Mar 26, 2025

Choose a reason for hiding this comment

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

@ditman we already have those flutter drive tests declared in example/integration_test/wakelock_plus_test.dart. I guess the question is whether these are the tests that should be run instead of the above ones?

Copy link

@ditman ditman Mar 26, 2025

Choose a reason for hiding this comment

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

Right, we also have some flutter test --platform=chrome tests in flutter/packages.

I'd recommend using either flutter test or flutter drive to run the tests. I'm not even sure what this program would return if the tests failed? (is this usable for CI, for example?)

Copy link
Collaborator

@diegotori diegotori Mar 26, 2025

Choose a reason for hiding this comment

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

@ditman Looks like even after porting this test to use flutter drive, it's still complaining about the requesting page not being visible.

@TestOn('browser')
library;

import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:wakelock_plus/src/wakelock_plus_web_plugin.dart';
import 'package:wakelock_plus/wakelock_plus.dart';
// ignore: depend_on_referenced_packages
import 'package:wakelock_plus_platform_interface/wakelock_plus_platform_interface.dart';

///
/// Run these tests with:
///   flutter run -d chrome test/wakelock_plus_web_plugin_test.dart
///
void main() {
  IntegrationTestWidgetsFlutterBinding.ensureInitialized();
  group('$WakelockPlusWebPlugin', () {
    setUpAll(() async {
      WakelockPlusPlatformInterface.instance = WakelockPlusWebPlugin();
    });

    tearDown(() async {
      await WakelockPlus.disable();
    });

    testWidgets('$WakelockPlusWebPlugin set as default instance',
        (tester) async {
      expect(
          WakelockPlusPlatformInterface.instance, isA<WakelockPlusWebPlugin>());
    });

    testWidgets('initially disabled', (tester) async {
      expect(WakelockPlus.enabled, completion(isFalse));
    });

    testWidgets('enable', (tester) async {
      await WakelockPlus.enable();
      expect(WakelockPlus.enabled, completion(isTrue));
    });

    testWidgets('enable more than once', (tester) async {
      await WakelockPlus.enable();
      await WakelockPlus.enable();
      await WakelockPlus.enable();
      expect(WakelockPlus.enabled, completion(isTrue));
    });

    testWidgets('disable', (tester) async {
      await WakelockPlus.enable();
      await WakelockPlus.disable();
      expect(WakelockPlus.enabled, completion(isFalse));
    });

    testWidgets('disable more than once', (tester) async {
      await WakelockPlus.enable();
      await WakelockPlus.disable();
      await WakelockPlus.disable();
      await WakelockPlus.disable();
      expect(WakelockPlus.enabled, completion(isFalse));
    });

    testWidgets('toggle', (tester) async {
      await WakelockPlus.toggle(enable: true);
      expect(WakelockPlus.enabled, completion(isTrue));

      await WakelockPlus.toggle(enable: false);
      expect(WakelockPlus.enabled, completion(isFalse));
    });
  });
}

Run with while in the example folder:

flutter drive -d web-server --web-port 7357 --browser-name chrome --driver test_driver/integration_test.dart --target integration_test/wakelock_plus_web_plugin_test.dart

Copy link
Collaborator

@diegotori diegotori Mar 26, 2025

Choose a reason for hiding this comment

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

Bottom line, what I'm looking for is an integration test that runs on a browser, that can also be spun up in a CI context. Not sure which one is the right approach, since the above is not working at all when using chromedriver.

Copy link
Collaborator

@diegotori diegotori Mar 26, 2025

Choose a reason for hiding this comment

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

@ditman looks like it's related to this issue when running flutter test --platform=chrome on wakelock_plus_web_plugin_test.dart in the root test folder.

Not sure what can be done if asset support isn't present when running the tests against a browser.

Copy link

@ditman ditman Mar 26, 2025

Choose a reason for hiding this comment

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

I suspect the problem is that the WakelockPlus calls must happen in the application under test, and not the test code itself.

If you inject a test app that has a button that enables/disables wakelock, and you trigger the button through the test driver, this should work, and assets would be available.

I think the code here is attempting to run Wakelock in the test runner code, not the application under test? :)

Copy link

@ditman ditman Mar 26, 2025

Choose a reason for hiding this comment

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

(I don't have time to try my solution right now, but I think the migration to an integration test with flutter driver might be inescapable if we want to test app assets)

Copy link
Collaborator

@diegotori diegotori Mar 26, 2025

Choose a reason for hiding this comment

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

So what you're saying is that I should instead inject the Example App, get a lock on the button that toggles the wakelock, and then make assertions from there?

Copy link

@ditman ditman Mar 27, 2025

Choose a reason for hiding this comment

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

Yes, that's what I'm saying... What I'm not sure now is that the assets are going to be available when the example app runs in flutter drive mode, as you're saying (because your test looks all right to me).

As a reference, a few examples of tests that "inject an app and then look at the DOM of the page or something else" to do integration testing:

Neither of those use assets, though.

Here's an example of an integration test that uses assets:

///
void main() {
group('$WakelockPlusWebPlugin', () {
setUpAll(() async {
// todo: the web tests do not work as the JS library import does not work.
// todo: the web tests do not work as the JS library import does not work when using flutter run test --platform chrome.
WakelockPlusPlatformInterface.instance = WakelockPlusWebPlugin();
});

Expand All @@ -24,16 +28,23 @@ void main() {

test('enable', () async {
await WakelockPlus.enable();
// Wait a bit for web to enable the wakelock
await Future.delayed(const Duration(milliseconds: 50));
expect(WakelockPlus.enabled, completion(isTrue));
});

test('disable', () async {
await WakelockPlus.enable();
// Wait a bit for web to enable the wakelock
await Future.delayed(const Duration(milliseconds: 50));
await WakelockPlus.disable();
expect(WakelockPlus.enabled, completion(isFalse));
});

test('toggle', () async {
await WakelockPlus.toggle(enable: true);
// Wait a bit for web to enable the wakelock
await Future.delayed(const Duration(milliseconds: 50));
expect(WakelockPlus.enabled, completion(isTrue));

await WakelockPlus.toggle(enable: false);
Expand Down
Loading