Skip to content

Delegate to LoadStratgy for module information #925

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

Merged
Parent: Update README.md to use `dart pub` instead of `pub`
merged 6 commits into from
Mar 18, 2020
Merged

Delegate to LoadStratgy for module information #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a47f2f
Delegate to LoadStratgy for module information
grouma Mar 17, 2020
3db74e5
fill out frontend
grouma Mar 17, 2020
985ff28
build
grouma Mar 17, 2020
2a6531e
fix frontend server tests
grouma Mar 17, 2020
f7c73c6
fix test
grouma Mar 18, 2020
ac8d8dc
fix readme
grouma Mar 18, 2020
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
7 changes: 7 additions & 0 deletions dwds/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 3.0.0-dev

**Breaking Changes:**
- Delegate to the `LoadStrategy` for module information:
- moduleId -> serverPath
- serverPath -> moduleId

## 2.0.1

- Fix an issue where we would return prematurely during a `hotRestart`.
Expand Down
17 changes: 5 additions & 12 deletions dwds/lib/src/debugging/location.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:path/path.dart' as p;
import 'package:source_maps/parser.dart';
import 'package:source_maps/source_maps.dart';

import '../loaders/strategy.dart';
import '../readers/asset_reader.dart';
import '../utilities/dart_uri.dart';
import 'modules.dart';
Expand Down Expand Up @@ -227,20 +228,13 @@ class Locations {
if (_moduleToLocations[module] != null) return _moduleToLocations[module];
var result = <Location>{};
if (module?.isEmpty ?? true) return _moduleToLocations[module] = result;
var moduleExtension = await _modules.moduleExtension;
var modulePath = '$module$moduleExtension';
if (modulePath.endsWith('dart_sdk.js') ||
modulePath.endsWith('dart_sdk.ddk.js') ||
// .lib.js extensions come from frontend server
modulePath.endsWith('dart_sdk.lib.js')) {
if (module.endsWith('dart_sdk')) {
return result;
}

modulePath = _modules.adjustForRoot(modulePath);
var scriptLocation = p.url.dirname(modulePath);

var modulePath = globalLoadStrategy.serverPathForModule(module);
var sourceMapContents =
await _assetReader.sourceMapContents('$modulePath.map');
var scriptLocation = p.url.dirname('/$modulePath');
if (sourceMapContents == null) return result;
var scriptId = await _modules.scriptIdForModule(module);
if (scriptId == null) return result;
Expand All @@ -259,8 +253,7 @@ class Locations {
var relativeSegments = p.split(mapping.urls[index]);
var path = p.url
.normalize(p.url.joinAll([scriptLocation, ...relativeSegments]));
var uri = _modules.adjustForRoot(path);
var dartUri = DartUri('/$uri', _root);
var dartUri = DartUri(path, _root);
result.add(Location.from(
scriptId,
lineEntry,
Expand Down
143 changes: 12 additions & 131 deletions dwds/lib/src/debugging/modules.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@

import 'dart:async';

import 'package:dwds/src/debugging/execution_context.dart';
import 'package:path/path.dart' as p;

import '../debugging/execution_context.dart';
import '../loaders/strategy.dart';
import '../utilities/dart_uri.dart';
import '../utilities/shared.dart';
Expand All @@ -29,16 +27,11 @@ class Modules {
// The module to corresponding Chrome script ID.
final _moduleToScriptId = <String, String>{};

final _moduleExtensionCompleter = Completer<String>();

Modules(this._remoteDebugger, String root, this._executionContext)
: _root = root == '' ? '/' : root;

/// Completes with the module extension i.e. `.ddc.js` or `.ddk.js`.
///
/// We use the script parsed events from Chrome to determine this information.
// TODO(grouma) - Do something better here.
Future<String> get moduleExtension => _moduleExtensionCompleter.future;
Modules(
this._remoteDebugger,
String root,
this._executionContext,
) : _root = root == '' ? '/' : root;

/// Initializes the mapping from source to module.
///
Expand Down Expand Up @@ -82,88 +75,18 @@ class Modules {
/// Checks if the [url] correspond to a module and stores meta data.
Future<Null> noteModule(String url, String scriptId) async {
var path = Uri.parse(url).path;
if (path == null ||
!(path.endsWith('.ddc.js') ||
path.endsWith('.ddk.js') ||
path.endsWith('.lib.js'))) {
return;
}

// TODO(grouma) - This is wonky. Find a better way.
if (!_moduleExtensionCompleter.isCompleted) {
if (path.endsWith('.ddc.js')) {
_moduleExtensionCompleter.complete('.ddc.js');
} else if (path.endsWith('.ddk.js')) {
_moduleExtensionCompleter.complete('.ddk.js');
} else {
_moduleExtensionCompleter.complete('.lib.js');
}
}

// TODO(annagrin): redirect modulePath->moduleName query to load strategy
//
// The code below is trying to guess the module name from js module path
// by assuming we can find a dart server path with a matching name located
// at the same server directory. Then it uses source->moduleName map to get
// the module name.
// [issue #917](https://github.com/dart-lang/webdev/issues/917)
// [issue #910](https://github.com/dart-lang/webdev/issues/910)
var serverPath = _jsModulePathToServerPath(path);
var module = await moduleForSource(serverPath);

if (path == null) return;
var module = globalLoadStrategy.moduleForServerPath(path);
if (module == null) return;
_scriptIdToModule[scriptId] = module;
_moduleToScriptId[module] = scriptId;
}

String _jsModulePathToServerPath(String path) {
// remove extensions, such as '.ddc.js'
var serverPath = p.withoutExtension(p.withoutExtension(path));
// server path does not contain leading '/'
serverPath =
serverPath.startsWith('/') ? serverPath.substring(1) : serverPath;
// server path has '.dart' extension
serverPath = serverPath.endsWith('.dart') ? serverPath : '$serverPath.dart';
// server path should be relative to the asset server root
serverPath = adjustForRoot(serverPath);
return DartUri('/$serverPath', _root).serverPath;
}

/// Make path relative to the asset server's serving root.
///
/// Remove the asset server root directory for non-package files, if any,
/// but do not remove the _root, which is the directory off the
/// asset server root. This is needed to produce paths that will be used
/// in requests to the asset server, such as dart script paths, dart
/// locations, or source map paths. Asset server is serving from the asset
/// server root directory, so it expects the requests to be relative to it.
// Note: This is a temporary workaround until we solve inconsistencies in
// different configurations by introducing module name and path translation
// interfaces between compiler, asset server, and the debugger.
// TODO(annagrin): module interface
// [issue #910](https://github.com/dart-lang/webdev/issues/910)
String adjustForRoot(String path) {
// path == 'dir/main.dart' => pathRoot == 'dir'
// path == 'main.dart' => pathRoot == '.'
var segments = p.split(path);
var pathRoot = p.split(p.dirname(path))[0];

// _root == 'http:/localhost:port/dir/index.html' => indexRoot == 'dir'
// _root == 'http:/localhost:port/index.html' => indexRoot == '.'
var indexPath = Uri.parse(_root).path.substring(1);
var indexRoot = p.split(p.dirname(indexPath))[0];

// remove the root from path only if not equal to packages or indexRoot
var result = pathRoot == 'packages' || pathRoot == indexRoot
// Module paths are consistent across platforms so join with a
// forward slash.
? p.url.joinAll(segments)
: p.url.joinAll(segments.skip(1));
return result;
}

/// Initializes [_sourceToModule].
Future<void> _initializeMapping() async {
if (_moduleCompleter.isCompleted) return;
// TODO(grouma) - We should talk to the compiler directly to greatly
// improve the performance here.
var expression = '''
(function() {
var dart = ${globalLoadStrategy.loadModuleSnippet}('dart_sdk').dart;
Expand All @@ -188,51 +111,9 @@ class Modules {
for (var dartScript in value.keys) {
if (!dartScript.endsWith('.dart')) continue;
var serverPath = DartUri(dartScript, _root).serverPath;

// get module name from module Uri
// Note: This is a temporary workaround until we solve inconsistencies
// in different configurations by introducing module name and path
// translation interfaces between compiler, asset server, and the
// debugger.
// TODO(annagrin): module interface
// [issue #910](https://github.com/dart-lang/webdev/issues/910)
var moduleUri = Uri.parse(value[dartScript] as String);
var module = _moduleFor(moduleUri.path);
_sourceToModule[serverPath] = module;
_sourceToModule[serverPath] = value[dartScript] as String;
_sourceToLibrary[serverPath] = Uri.parse(dartScript);
}
_moduleCompleter.complete();
}

/// Returns the module for the provided path.
///
/// Module are of the following form:
///
/// packages/foo/bar/module
/// some/root/bar/module
///
String _moduleFor(String path, {bool skipRoot}) {
path = '/$path';
skipRoot ??= false;
var result = '';
if (path.contains('/packages/')) {
result = 'packages/${path.split('/packages/').last}';
} else if (path.contains('/lib/')) {
var splitModule = path.split('/lib/').first.substring(1).split('/');
// Special case third_party/dart for Google3.
if (path.startsWith('/third_party/dart/')) {
splitModule = splitModule.skip(2).toList();
}
result = 'packages/${splitModule.join(".")}/${p.basename(path)}';
} else if (path.contains('/google3/')) {
result = path.split('/google3/').last;
} else if (path.startsWith('/')) {
path = path.substring(1);
if (skipRoot) {
path = path.split('/').skip(1).join('/');
}
result = path;
}
return result;
}
}
2 changes: 1 addition & 1 deletion dwds/lib/src/dwds_vm_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class DwdsVmClient {
}
}
// Only return success after the isolate has fully started.
var stream = client.onEvent('Isolate');
var stream = chromeProxyService.onEvent('Isolate');
await stream.firstWhere((event) => event.kind == EventKind.kIsolateStart);
return {'result': Success().toJson()};
});
Expand Down
42 changes: 38 additions & 4 deletions dwds/lib/src/loaders/build_runner_require.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,32 @@
import 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as p;
import 'package:shelf/shelf.dart';

import 'require.dart';
import 'strategy.dart';

/// Provides a [RequireStrategy] suitable for use with `package:build_runner`.
class BuildRunnerRequireStrategyProvider {
final _extension = '.ddc';
final Handler _assetHandler;
final ReloadConfiguration _configuration;
final _serverPathToModule = <String, String>{};

RequireStrategy _requireStrategy;

BuildRunnerRequireStrategyProvider(this._assetHandler, this._configuration);

RequireStrategy get strategy => _requireStrategy ??= RequireStrategy(
_configuration, '.ddc', _moduleProvider, _digestsProvider);
_configuration,
_extension,
_moduleProvider,
_digestsProvider,
_moduleForServerPath,
_serverPathForModule,
_serverPathForAppUri,
);

Future<Map<String, String>> _digestsProvider(String entrypoint) async {
var digestsPath = entrypoint.replaceAll('.dart.bootstrap.js', '.digests');
Expand Down Expand Up @@ -50,8 +60,32 @@ class BuildRunnerRequireStrategyProvider {

Future<Map<String, String>> _moduleProvider(String entrypoint) async {
var digests = await _digestsProvider(entrypoint);
return {
for (var moduleId in digests.keys) moduleId: _serverPath(moduleId),
};
var result = <String, String>{};
_serverPathToModule.clear();
for (var moduleId in digests.keys) {
var serverPath = _serverPath(moduleId);
_serverPathToModule[serverPath] = moduleId;
result[moduleId] = serverPath;
}
return result;
}

String _moduleForServerPath(String serverPath) {
if (!serverPath.endsWith('$_extension.js')) return null;
serverPath =
serverPath.startsWith('/') ? serverPath.substring(1) : serverPath;
// Remove the .js from the path.
serverPath = p.withoutExtension(serverPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

a bit surprising we don't remove the full extension here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Our server path is of the form packages/path/path.ddc. It's commented above so it shouldn't be surprising. Should I add more comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is just repeating the code and not explaining why it is the way it is :D. A follow-up to clarify would be good but not required.

Copy link
Member Author

Choose a reason for hiding this comment

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

I meant here: https://github.com/dart-lang/webdev/blob/master/dwds/lib/src/loaders/build_runner_require.dart#L52

return _serverPathToModule[serverPath];
}

String _serverPathForModule(String module) => '${_serverPath(module)}.js';

String _serverPathForAppUri(String appUri) {
if (appUri.startsWith('org-dartlang-app:')) {
// We skip the root from which we are serving.
return Uri.parse(appUri).pathSegments.skip(1).join('/');
}
return null;
}
}
41 changes: 28 additions & 13 deletions dwds/lib/src/loaders/frontend_server_require.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,52 @@ import 'package:dwds/dwds.dart';
class FrontendServerRequireStrategyProvider {
final ReloadConfiguration _configuration;
final Iterable<String> _modules;
final _extension = '.lib.js';
RequireStrategy _requireStrategy;

FrontendServerRequireStrategyProvider(this._modules, this._configuration);

RequireStrategy get strategy => _requireStrategy ??= RequireStrategy(
_configuration, '.lib.js', _moduleProvider, _digestsProvider);
_configuration,
_extension,
_moduleProvider,
_digestsProvider,
_moduleForServerPath,
_serverPathForModule,
_serverPathForAppUri,
);

Future<Map<String, String>> _digestsProvider(String entrypoint) async {
// TODO(grouma) - provide actual digests.
return {};
}

Future<Map<String, String>> _moduleProvider(String entrypoint) async {
final modulePaths = <String, String>{};
for (var module in _modules) {
// We are currently 'guessing' module names from js module paths,
// which is not reliable.
// example:
// module: /web/main.dart.lib.js'
// name: web/main.dart
// path: web/main.dart.lib
// Note: This is a temporary workaround until we solve inconsistencies
// in different configurations by introducing module name and path
// translation interfaces between compiler, asset server, and the
// debugger.
// TODO(annagrin): module interface
// [issue #910](https://github.com/dart-lang/webdev/issues/910)
module = module.startsWith('/') ? module.substring(1) : module;
var name = module.replaceAll('.lib.js', '');
var path = module.replaceAll('.js', '');
modulePaths[name] = path;
}
return modulePaths;
}

String _moduleForServerPath(String serverPath) {
if (serverPath.endsWith('.lib.js')) {
serverPath =
serverPath.startsWith('/') ? serverPath.substring(1) : serverPath;
return serverPath.replaceAll('.lib.js', '');
}
return null;
}

String _serverPathForModule(String module) => '$module.lib.js';

String _serverPathForAppUri(String appUri) {
if (appUri.startsWith('org-dartlang-app:')) {
return Uri.parse(appUri).path.substring(1);
}
return null;
}
}
Loading