Skip to content

Commit

Permalink
Fix relative imports in wasm loader (#3752)
Browse files Browse the repository at this point in the history
Currently, the loader script emitted by `build_web_compilers` resolves URLs against the base-URL of the active document. This breaks when the entrypoint script is not in the same directory as the page, e.g. in

```
web/
  index.html
  src/
    app.dart
```

To fix this, this PR changes the logic to resolve URLs against the loader script (which would be `/src/app.dart.js` in the example, properly resolving the other dart2wasm/dart2js components).
I've also added an integration test using a `<script>` tag pointing to a subdirectory to make sure that this is working now.

Closes #3751
  • Loading branch information
simolus3 authored Sep 25, 2024
1 parent 16da6fb commit 564250e
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 13 deletions.
3 changes: 3 additions & 0 deletions _test/build.both.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ targets:
- test/hello_world_deferred_test.dart.browser_test.dart
- test/hello_world_custom_html_test.dart
- test/hello_world_custom_html_test.dart.browser_test.dart
- test/subdir_source_test.dart
- test/subdir_source_test.dart.browser_test.dart
- test/other_test.dart.browser_test.dart
- test/sub-dir/subdir_source.dart
- test/sub-dir/subdir_test.dart
- test/sub-dir/subdir_test.dart.browser_test.dart
3 changes: 3 additions & 0 deletions _test/build.dart2js.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ targets:
- test/hello_world_deferred_test.dart.browser_test.dart
- test/hello_world_custom_html_test.dart
- test/hello_world_custom_html_test.dart.browser_test.dart
- test/subdir_source_test.dart
- test/subdir_source_test.dart.browser_test.dart
- test/other_test.dart.browser_test.dart
- test/sub-dir/subdir_source.dart
- test/sub-dir/subdir_test.dart
- test/sub-dir/subdir_test.dart.browser_test.dart
3 changes: 3 additions & 0 deletions _test/build.dart2wasm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ targets:
- test/hello_world_deferred_test.dart.browser_test.dart
- test/hello_world_custom_html_test.dart
- test/hello_world_custom_html_test.dart.browser_test.dart
- test/subdir_source_test.dart
- test/subdir_source_test.dart.browser_test.dart
- test/other_test.dart.browser_test.dart
- test/sub-dir/subdir_source.dart
- test/sub-dir/subdir_test.dart
- test/sub-dir/subdir_test.dart.browser_test.dart
2 changes: 2 additions & 0 deletions _test/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ targets:
- test/hello_world_deferred_test.dart.browser_test.dart
- test/hello_world_custom_html_test.dart.browser_test.dart
- test/other_test.dart.browser_test.dart
- test/subdir_source_test.dart.browser_test.dart
- test/sub-dir/subdir_source.dart
- test/sub-dir/subdir_test.dart.browser_test.dart
2 changes: 1 addition & 1 deletion _test/dart_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ timeout: 16x
concurrency: 1

tags:
integration:
# This tag is used for integration tests - we don't need special options at the
# moment, but want to avoid warnings from the test runner about using undefined
# targets.
integration:

define_platforms:
chrome_without_wasm:
Expand Down
2 changes: 1 addition & 1 deletion _test/mono_pkg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ stages:
os: linux
- unit_test:
- command: dart run build_runner test -- -p chrome --test-randomize-ordering-seed=random
# TODO(https://github.com/dart-lang/build/issues/3423): restore this on windows
# TODO(https://github.com/dart-lang/build/issues/3423): restore this on windows
- command: dart run build_runner test -- -p vm test/configurable_uri_test.dart --test-randomize-ordering-seed=random
os: linux
- e2e_test:
Expand Down
10 changes: 10 additions & 0 deletions _test/test/sub-dir/subdir_source.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:js_interop';
import 'dart:js_interop_unsafe';

void main() {
globalContext['otherScriptLoaded'] = true.toJS;
}
41 changes: 41 additions & 0 deletions _test/test/subdir_source_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
@TestOn('browser')
library;

import 'dart:js_interop';
import 'dart:js_interop_unsafe';

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

void main() {
final wasCompiledWithDdc = globalContext.has('define');

test(
'did load script from a subdirectory',
() async {
final scriptTag = document.createElement('script') as HTMLScriptElement;
scriptTag
..type = 'application/javascript'
..src = 'sub-dir/subdir_source.dart.js';
document.head!.append(scriptTag);

await Future.any([
scriptTag.onLoad.first,
scriptTag.onError.first
.then((_) => fail('Script from sub directory failed to load'))
]);

await pumpEventQueue();

// `sub-dir/subdir_source.dart.js` should have set the `otherScriptLoader`
// propery.
expect(globalContext.has('otherScriptLoaded'), isTrue);
},
skip: wasCompiledWithDdc
? 'This requires multiple Dart entrypoints, which appears to break DDC'
: null,
);
}
4 changes: 4 additions & 0 deletions build_web_compilers/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 4.1.0-wip

- Fix loading compiled modules from subdirectories.

## 4.1.0-beta.0

- Support compiling to WebAssembly by using `dart2wasm` as a value for the
Expand Down
20 changes: 14 additions & 6 deletions build_web_compilers/lib/src/web_entrypoint_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,15 @@ class WebEntrypointBuilder implements Builder {
var jsCompiler = options.optionsFor(WebCompiler.Dart2Js) ??
options.optionsFor(WebCompiler.DartDevc);

var loaderResult = StringBuffer('''(async () => {
var loaderResult = StringBuffer('''
(async () => {
const thisScript = document.currentScript;
function relativeURL(ref) {
const base = thisScript?.src ?? document.baseURI;
return new URL(ref, base).toString();
}
''');

// If we're compiling to JS, start a feature detection to prefer wasm but
Expand All @@ -355,19 +363,19 @@ if (supportsWasmGC()) {
}

loaderResult.writeln('''
let { instantiate, invoke } = await import("./$basename${wasmCompiler.extension}");
let { compileStreaming } = await import("./$basename${wasmCompiler.extension}");
let modulePromise = WebAssembly.compileStreaming(fetch("$basename.wasm"));
let instantiated = await instantiate(modulePromise, {});
invoke(instantiated, []);
let app = await compileStreaming(fetch(relativeURL("$basename.wasm")));
let module = await app.instantiate({});
module.invokeMain();
''');

if (jsCompiler != null) {
loaderResult.writeln('''
} else {
const scriptTag = document.createElement("script");
scriptTag.type = "application/javascript";
scriptTag.src = new URL("./$basename${jsCompiler.extension}", document.baseURI).toString();
scriptTag.src = relativeURL("./$basename${jsCompiler.extension}");
document.head.append(scriptTag);
}
''');
Expand Down
2 changes: 1 addition & 1 deletion build_web_compilers/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: build_web_compilers
version: 4.1.0-beta.0
version: 4.1.0-wip
description: Builder implementations wrapping the dart2js and DDC compilers.
repository: https://github.com/dart-lang/build/tree/master/build_web_compilers
# This package can't be part of the workspace because it requires a very recent
Expand Down
7 changes: 3 additions & 4 deletions build_web_compilers/test/dart2wasm_bootstrap_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ void main() {
outputs: {
'a|web/index.mjs': anything,
'a|web/index.wasm': anything,
'a|web/index.dart.js':
decodedMatches(contains('WebAssembly.compileStreaming')),
'a|web/index.dart.js': decodedMatches(contains('compileStreaming')),
},
);
});
Expand All @@ -69,9 +68,9 @@ void main() {
stringContainsInOrder(
[
'if (supportsWasmGC())',
'WebAssembly.compileStreaming',
'compileStreaming',
'else',
'scriptTag.src = new URL("./index.dart2js.js", document.baseURI).toString();'
'scriptTag.src = relativeURL("./index.dart2js.js");'
],
),
),
Expand Down

0 comments on commit 564250e

Please sign in to comment.