From 6b584dad2d06ad81bc49e0fddcc678b183ab4e17 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Mon, 13 Jun 2022 16:38:01 -0700 Subject: [PATCH 1/8] Removed dependecies on dwds.dart inside dwds code --- dwds/lib/src/debugging/classes.dart | 2 +- dwds/lib/src/handlers/dev_handler.dart | 10 +++++++++- dwds/lib/src/handlers/injector.dart | 1 - dwds/lib/src/loaders/build_runner_require.dart | 5 ++++- dwds/lib/src/loaders/frontend_server_require.dart | 5 ++++- dwds/lib/src/loaders/legacy.dart | 5 ++++- dwds/lib/src/loaders/require.dart | 5 ++++- dwds/lib/src/loaders/strategy.dart | 4 +++- dwds/lib/src/readers/frontend_server_asset_reader.dart | 3 +-- dwds/lib/src/services/chrome_proxy_service.dart | 6 +++++- dwds/lib/src/services/debug_service.dart | 6 +++++- dwds/lib/src/utilities/sdk_configuration.dart | 2 +- 12 files changed, 41 insertions(+), 13 deletions(-) diff --git a/dwds/lib/src/debugging/classes.dart b/dwds/lib/src/debugging/classes.dart index f9fdfd10a..112ad8994 100644 --- a/dwds/lib/src/debugging/classes.dart +++ b/dwds/lib/src/debugging/classes.dart @@ -7,7 +7,7 @@ import 'package:vm_service/vm_service.dart'; import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; -import '../../dwds.dart' show ChromeDebugException; +import '../../src/services/chrome_debug_exception.dart'; import '../loaders/strategy.dart'; import '../utilities/domain.dart'; import '../utilities/shared.dart'; diff --git a/dwds/lib/src/handlers/dev_handler.dart b/dwds/lib/src/handlers/dev_handler.dart index 6445ec495..37f7c0ea8 100644 --- a/dwds/lib/src/handlers/dev_handler.dart +++ b/dwds/lib/src/handlers/dev_handler.dart @@ -21,15 +21,23 @@ import '../../data/error_response.dart'; import '../../data/isolate_events.dart'; import '../../data/register_event.dart'; import '../../data/serializers.dart'; -import '../../dwds.dart'; + +import '../connections/app_connection.dart'; +import '../connections/debug_connection.dart'; import '../debugging/execution_context.dart'; import '../debugging/remote_debugger.dart'; import '../debugging/webkit_debugger.dart'; import '../dwds_vm_client.dart'; import '../events.dart'; +import '../handlers/socket_connections.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../servers/devtools.dart'; import '../servers/extension_backend.dart'; import '../services/app_debug_services.dart'; import '../services/debug_service.dart'; +import '../services/expression_compiler.dart'; +import '../utilities/sdk_configuration.dart'; import 'injector.dart'; /// When enabled, this logs VM service protocol and Chrome debug protocol diff --git a/dwds/lib/src/handlers/injector.dart b/dwds/lib/src/handlers/injector.dart index 536464373..dfdff18de 100644 --- a/dwds/lib/src/handlers/injector.dart +++ b/dwds/lib/src/handlers/injector.dart @@ -13,7 +13,6 @@ import 'package:crypto/crypto.dart'; import 'package:logging/logging.dart'; import 'package:shelf/shelf.dart'; -import '../../dwds.dart'; import '../loaders/strategy.dart'; import '../version.dart'; diff --git a/dwds/lib/src/loaders/build_runner_require.dart b/dwds/lib/src/loaders/build_runner_require.dart index a03ea679a..ddcf66c9f 100644 --- a/dwds/lib/src/loaders/build_runner_require.dart +++ b/dwds/lib/src/loaders/build_runner_require.dart @@ -10,7 +10,10 @@ import 'dart:io'; import 'package:path/path.dart' as p; import 'package:shelf/shelf.dart'; -import '../../dwds.dart'; +import '../debugging/metadata/provider.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; import 'require.dart'; /// Provides a [RequireStrategy] suitable for use with `package:build_runner`. diff --git a/dwds/lib/src/loaders/frontend_server_require.dart b/dwds/lib/src/loaders/frontend_server_require.dart index 925714486..d3b7eb84f 100644 --- a/dwds/lib/src/loaders/frontend_server_require.dart +++ b/dwds/lib/src/loaders/frontend_server_require.dart @@ -6,7 +6,10 @@ import 'package:path/path.dart' as p; -import '../../dwds.dart'; +import '../debugging/metadata/provider.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; import 'require.dart'; /// Provides a [RequireStrategy] suitable for use with Frontend Server. diff --git a/dwds/lib/src/loaders/legacy.dart b/dwds/lib/src/loaders/legacy.dart index df4c02206..f910711b7 100644 --- a/dwds/lib/src/loaders/legacy.dart +++ b/dwds/lib/src/loaders/legacy.dart @@ -6,7 +6,10 @@ import 'package:shelf/shelf.dart'; -import '../../dwds.dart'; +import '../debugging/metadata/provider.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; /// A load strategy for the legacy module system. class LegacyStrategy extends LoadStrategy { diff --git a/dwds/lib/src/loaders/require.dart b/dwds/lib/src/loaders/require.dart index 7058e1561..30413d7af 100644 --- a/dwds/lib/src/loaders/require.dart +++ b/dwds/lib/src/loaders/require.dart @@ -9,7 +9,10 @@ import 'dart:convert'; import 'package:path/path.dart' as p; import 'package:shelf/shelf.dart'; -import '../../dwds.dart'; +import '../debugging/metadata/provider.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; /// Find the path we are serving from the url. /// diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart index a861ff237..c98ec8d45 100644 --- a/dwds/lib/src/loaders/strategy.dart +++ b/dwds/lib/src/loaders/strategy.dart @@ -6,7 +6,9 @@ import 'package:shelf/shelf.dart'; -import '../../dwds.dart'; +import '../debugging/metadata/provider.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; LoadStrategy _globalLoadStrategy; diff --git a/dwds/lib/src/readers/frontend_server_asset_reader.dart b/dwds/lib/src/readers/frontend_server_asset_reader.dart index 5a4ba09ec..31326f8ba 100644 --- a/dwds/lib/src/readers/frontend_server_asset_reader.dart +++ b/dwds/lib/src/readers/frontend_server_asset_reader.dart @@ -10,8 +10,7 @@ import 'dart:io'; import 'package:package_config/package_config.dart'; import 'package:path/path.dart' as p; -import '../../dwds.dart'; -import 'asset_reader.dart'; +import '../readers/asset_reader.dart'; /// A reader for Dart sources and related source maps provided by the Frontend /// Server. diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index f8de6e86e..7bc8aa21a 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -15,7 +15,7 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; import '../../data/debug_event.dart'; import '../../data/register_event.dart'; -import '../../dwds.dart'; +import '../connections/app_connection.dart'; import '../debugging/debugger.dart'; import '../debugging/execution_context.dart'; import '../debugging/inspector.dart'; @@ -26,7 +26,11 @@ import '../debugging/remote_debugger.dart'; import '../debugging/skip_list.dart'; import '../events.dart'; import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/chrome_debug_exception.dart'; +import '../services/expression_compiler.dart'; import '../utilities/dart_uri.dart'; +import '../utilities/sdk_configuration.dart'; import '../utilities/shared.dart'; import 'expression_evaluator.dart'; diff --git a/dwds/lib/src/services/debug_service.dart b/dwds/lib/src/services/debug_service.dart index cf5d1409e..9df8d705c 100644 --- a/dwds/lib/src/services/debug_service.dart +++ b/dwds/lib/src/services/debug_service.dart @@ -19,11 +19,15 @@ import 'package:sse/server/sse_handler.dart'; import 'package:vm_service/vm_service.dart'; import 'package:web_socket_channel/web_socket_channel.dart'; -import '../../dwds.dart'; +import '../connections/app_connection.dart'; +import '../loaders/strategy.dart'; +import '../readers/asset_reader.dart'; +import '../services/expression_compiler.dart'; import '../debugging/execution_context.dart'; import '../debugging/remote_debugger.dart'; import '../events.dart'; import '../utilities/shared.dart'; +import '../utilities/sdk_configuration.dart'; import 'chrome_proxy_service.dart'; bool _acceptNewConnections = true; diff --git a/dwds/lib/src/utilities/sdk_configuration.dart b/dwds/lib/src/utilities/sdk_configuration.dart index eb650187e..0439e01e6 100644 --- a/dwds/lib/src/utilities/sdk_configuration.dart +++ b/dwds/lib/src/utilities/sdk_configuration.dart @@ -120,7 +120,7 @@ class SdkConfiguration { class DefaultSdkConfigurationProvider extends SdkConfigurationProvider { DefaultSdkConfigurationProvider(); - late SdkConfiguration _configuration = _create(); + late final SdkConfiguration _configuration = _create(); /// Create and validate configuration matching the default SDK layout. @override From 679dc2958ca78bc52d487b638cecd55b5ee4e1e1 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Tue, 14 Jun 2022 12:17:58 -0700 Subject: [PATCH 2/8] Migrate loaders and debugging/metadata directories to null safety --- dwds/lib/src/debugging/metadata/function.dart | 7 +- .../debugging/metadata/module_metadata.dart | 65 +++++++++++-------- dwds/lib/src/debugging/metadata/provider.dart | 7 +- dwds/lib/src/handlers/injector.dart | 4 +- .../lib/src/loaders/build_runner_require.dart | 58 +++++++++++------ .../src/loaders/frontend_server_require.dart | 54 ++++++++------- dwds/lib/src/loaders/legacy.dart | 8 +-- dwds/lib/src/loaders/require.dart | 14 ++-- dwds/lib/src/loaders/strategy.dart | 22 +++---- dwds/test/evaluate_common.dart | 1 + dwds/test/fixtures/context.dart | 1 + 11 files changed, 137 insertions(+), 104 deletions(-) diff --git a/dwds/lib/src/debugging/metadata/function.dart b/dwds/lib/src/debugging/metadata/function.dart index f252624dd..bc435a0b8 100644 --- a/dwds/lib/src/debugging/metadata/function.dart +++ b/dwds/lib/src/debugging/metadata/function.dart @@ -2,8 +2,6 @@ // 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:async'; -// @dart = 2.9 - import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'; import '../../loaders/strategy.dart'; @@ -43,8 +41,9 @@ class FunctionMetaData { response, evalContents: evalExpression, ); - var name = response.result['result']['value'] as String; - if (name.isEmpty) name = 'Closure'; + final name = response.result?['result']?['value'] as String?; + if (name == null) return FunctionMetaData(''); + if (name.isEmpty) return FunctionMetaData('Closure'); return FunctionMetaData(name); } } diff --git a/dwds/lib/src/debugging/metadata/module_metadata.dart b/dwds/lib/src/debugging/metadata/module_metadata.dart index 2bf2c693b..9f4ae4c30 100644 --- a/dwds/lib/src/debugging/metadata/module_metadata.dart +++ b/dwds/lib/src/debugging/metadata/module_metadata.dart @@ -2,8 +2,6 @@ // 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. -// @dart = 2.9 - /// Module metadata format version /// /// Module reader always creates the current version but is able to read @@ -62,25 +60,25 @@ class ModuleMetadataVersion { /// See: https://goto.google.com/dart-web-debugger-metadata class LibraryMetadata { /// Library name as defined in pubspec.yaml - final String name; + late final String name; /// Library importUri /// /// Example package:path/path.dart - final String importUri; + late final String importUri; /// All file uris from the library /// /// Can be relative paths to the directory of the fileUri - final List partUris; + late final List partUris; LibraryMetadata(this.name, this.importUri, this.partUris); - LibraryMetadata.fromJson(Map json) - : name = json['name'] as String, - importUri = json['importUri'] as String, - partUris = - List.castFrom(json['partUris'] as List); + LibraryMetadata.fromJson(Map json) { + name = _readRequiredField(json, 'name'); + importUri = _readRequiredField(json, 'importUri'); + partUris = _readOptionalList(json, 'partUris') ?? []; + } Map toJson() { return { @@ -98,35 +96,35 @@ class LibraryMetadata { /// See: https://goto.google.com/dart-web-debugger-metadata class ModuleMetadata { /// Metadata format version - String version; + late final String version; /// Module name /// /// Used as a name of the js module created by the compiler and /// as key to store and load modules in the debugger and the browser - final String name; + late final String name; /// Name of the function enclosing the module /// /// Used by debugger to determine the top dart scope - final String closureName; + late final String closureName; /// Source map uri - final String sourceMapUri; + late final String sourceMapUri; /// Module uri - final String moduleUri; + late final String moduleUri; /// True if the module corresponding to this metadata was compiled with sound /// null safety enabled. - final bool soundNullSafety; + late final bool soundNullSafety; final Map libraries = {}; ModuleMetadata(this.name, this.closureName, this.sourceMapUri, this.moduleUri, this.soundNullSafety, - {this.version}) { - version ??= ModuleMetadataVersion.current.version; + {String? ver}) { + version = ver ?? ModuleMetadataVersion.current.version; } /// Add [library] to metadata @@ -144,13 +142,13 @@ class ModuleMetadata { } } - ModuleMetadata.fromJson(Map json) - : version = json['version'] as String, - name = json['name'] as String, - closureName = json['closureName'] as String, - sourceMapUri = json['sourceMapUri'] as String, - moduleUri = json['moduleUri'] as String, - soundNullSafety = (json['soundNullSafety'] as bool) ?? false { + ModuleMetadata.fromJson(Map json) { + version = _readRequiredField(json, 'version'); + name = _readRequiredField(json, 'name'); + closureName = _readRequiredField(json, 'closureName'); + sourceMapUri = _readRequiredField(json, 'sourceMapUri'); + moduleUri = _readRequiredField(json, 'moduleUri'); + soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false; if (!ModuleMetadataVersion.current.isCompatibleWith(version) && !ModuleMetadataVersion.previous.isCompatibleWith(version)) { throw Exception('Unsupported metadata version $version. ' @@ -159,7 +157,7 @@ class ModuleMetadata { '\n ${ModuleMetadataVersion.previous.version}'); } - for (var l in json['libraries'] as List) { + for (var l in _readRequiredList(json, 'libraries')) { addLibrary(LibraryMetadata.fromJson(l as Map)); } } @@ -176,3 +174,18 @@ class ModuleMetadata { }; } } + +// TODO: format errors! +T _readRequiredField(Map json, String field) => + json[field]! as T; + +T? _readOptionalField(Map json, String field) => + json[field] as T?; + +List _readRequiredList(Map json, String field) => + List.castFrom(json[field] as List); + +List? _readOptionalList(Map json, String field) => + json.containsKey(field) + ? List.castFrom(json[field] as List) + : null; diff --git a/dwds/lib/src/debugging/metadata/provider.dart b/dwds/lib/src/debugging/metadata/provider.dart index 822ac4715..ae4696e5b 100644 --- a/dwds/lib/src/debugging/metadata/provider.dart +++ b/dwds/lib/src/debugging/metadata/provider.dart @@ -2,8 +2,6 @@ // 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:async'; -// @dart = 2.9 - import 'dart:async'; import 'dart:convert'; @@ -194,8 +192,7 @@ class MetadataProvider { _addSdkMetadata(); for (var contents in merged.split('\n')) { try { - if (contents == null || - contents.isEmpty || + if (contents.isEmpty || contents.startsWith('// intentionally empty:')) continue; final moduleJson = json.decode(contents); final metadata = @@ -237,7 +234,7 @@ class MetadataProvider { for (var path in library.partUris) { // Parts in metadata are relative to the library Uri directory. final partPath = p.url.join(p.dirname(library.importUri), path); - _scripts[library.importUri].add(partPath); + _scripts[library.importUri]!.add(partPath); _scriptToModule[partPath] = metadata.name; } } diff --git a/dwds/lib/src/handlers/injector.dart b/dwds/lib/src/handlers/injector.dart index dfdff18de..d0089a41f 100644 --- a/dwds/lib/src/handlers/injector.dart +++ b/dwds/lib/src/handlers/injector.dart @@ -123,7 +123,9 @@ class DwdsInjector { return response.change(body: body, headers: newHeaders); } else { final loadResponse = await _loadStrategy.handler(request); - if (loadResponse != null) return loadResponse; + if (loadResponse != null && loadResponse.statusCode != 404) { + return loadResponse; + } return innerHandler(request); } }; diff --git a/dwds/lib/src/loaders/build_runner_require.dart b/dwds/lib/src/loaders/build_runner_require.dart index ddcf66c9f..dbdda0a08 100644 --- a/dwds/lib/src/loaders/build_runner_require.dart +++ b/dwds/lib/src/loaders/build_runner_require.dart @@ -2,11 +2,10 @@ // 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. -// @dart = 2.9 - import 'dart:convert'; import 'dart:io'; +import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; import 'package:shelf/shelf.dart'; @@ -18,26 +17,28 @@ import 'require.dart'; /// Provides a [RequireStrategy] suitable for use with `package:build_runner`. class BuildRunnerRequireStrategyProvider { + final _logger = Logger('BuildRunnerRequireStrategyProvider'); + final Handler _assetHandler; final ReloadConfiguration _configuration; final AssetReader _assetReader; - RequireStrategy _requireStrategy; + late final RequireStrategy _requireStrategy = RequireStrategy( + _configuration, + _moduleProvider, + _digestsProvider, + _moduleForServerPath, + _serverPathForModule, + _sourceMapPathForModule, + _serverPathForAppUri, + _moduleInfoForProvider, + _assetReader, + ); BuildRunnerRequireStrategyProvider( this._assetHandler, this._configuration, this._assetReader); - RequireStrategy get strategy => _requireStrategy ??= RequireStrategy( - _configuration, - _moduleProvider, - _digestsProvider, - _moduleForServerPath, - _serverPathForModule, - _sourceMapPathForModule, - _serverPathForAppUri, - _moduleInfoForProvider, - _assetReader, - ); + RequireStrategy get strategy => _requireStrategy; Future> _digestsProvider( MetadataProvider metadataProvider) async { @@ -51,9 +52,18 @@ class BuildRunnerRequireStrategyProvider { throw StateError('Could not read digests at path: $digestsPath'); } final body = await response.readAsString(); + final digests = json.decode(body) as Map; + + for (final key in digests.keys) { + if (!modules.containsKey(key)) { + _logger.warning('Digest key $key is not a module name.'); + } + } + return { - for (var entry in (json.decode(body) as Map).entries) - modules[entry.key]: entry.value as String, + for (var entry in digests.entries) + if (modules.containsKey(entry.key)) + modules[entry.key]!: entry.value as String, }; } @@ -63,9 +73,17 @@ class BuildRunnerRequireStrategyProvider { MapEntry(key, stripTopLevelDirectory(removeJsExtension(value)))); Future _moduleForServerPath( - MetadataProvider metadataProvider, String serverPath) async => - (await metadataProvider.modulePathToModule).map((key, value) => MapEntry( - stripTopLevelDirectory(key), value))[relativizePath(serverPath)]; + MetadataProvider metadataProvider, String serverPath) async { + final modulePathToModule = await metadataProvider.modulePathToModule; + final relativePath = relativizePath(serverPath); + for (var e in modulePathToModule.entries) { + if (stripTopLevelDirectory(e.key) == relativePath) { + return e.value; + } + } + _logger.warning('No module for server path $serverPath'); + return ''; + } Future _serverPathForModule( MetadataProvider metadataProvider, String module) async { @@ -80,7 +98,7 @@ class BuildRunnerRequireStrategyProvider { return stripTopLevelDirectory(path); } - String _serverPathForAppUri(String appUri) { + 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('/'); diff --git a/dwds/lib/src/loaders/frontend_server_require.dart b/dwds/lib/src/loaders/frontend_server_require.dart index d3b7eb84f..9e4dce317 100644 --- a/dwds/lib/src/loaders/frontend_server_require.dart +++ b/dwds/lib/src/loaders/frontend_server_require.dart @@ -2,8 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.9 - +import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; import '../debugging/metadata/provider.dart'; @@ -14,36 +13,38 @@ import 'require.dart'; /// Provides a [RequireStrategy] suitable for use with Frontend Server. class FrontendServerRequireStrategyProvider { + final _logger = Logger('FrontendServerRequireStrategyProvider'); final ReloadConfiguration _configuration; final AssetReader _assetReader; final Future> Function() _digestsProvider; final String _basePath; - RequireStrategy _requireStrategy; + late final RequireStrategy _requireStrategy = RequireStrategy( + _configuration, + _moduleProvider, + (_) => _digestsProvider(), + _moduleForServerPath, + _serverPathForModule, + _sourceMapPathForModule, + _serverPathForAppUri, + _moduleInfoForProvider, + _assetReader, + ); FrontendServerRequireStrategyProvider(this._configuration, this._assetReader, this._digestsProvider, String basePath) - : _basePath = basePathForServerUri(basePath); + : _basePath = basePathForServerUri(basePath) { + _logger.info('Base path: $_basePath'); + } - RequireStrategy get strategy => _requireStrategy ??= RequireStrategy( - _configuration, - _moduleProvider, - (_) => _digestsProvider(), - _moduleForServerPath, - _serverPathForModule, - _sourceMapPathForModule, - _serverPathForAppUri, - _moduleInfoForProvider, - _assetReader, - ); + RequireStrategy get strategy => _requireStrategy; - String _removeBasePath(String path) => + String? _removeBasePath(String path) => path.startsWith(_basePath) ? path.substring(_basePath.length) : null; - String _addBasePath(String serverPath) => - _basePath == null || _basePath.isEmpty - ? relativizePath(serverPath) - : '$_basePath/${relativizePath(serverPath)}'; + String _addBasePath(String serverPath) => _basePath.isEmpty + ? relativizePath(serverPath) + : '$_basePath/${relativizePath(serverPath)}'; Future> _moduleProvider( MetadataProvider metadataProvider) async => @@ -53,7 +54,14 @@ class FrontendServerRequireStrategyProvider { Future _moduleForServerPath( MetadataProvider metadataProvider, String serverPath) async { final modulePathToModule = await metadataProvider.modulePathToModule; - return modulePathToModule[_removeBasePath(serverPath)]; + final relativeServerPath = _removeBasePath(serverPath); + if (relativeServerPath == null || + !modulePathToModule.containsKey(relativeServerPath)) { + _logger.warning( + 'No module found for server path: $serverPath. Server path is not under $_basePath.'); + return ''; + } + return modulePathToModule[relativeServerPath]!; } Future _serverPathForModule( @@ -64,7 +72,7 @@ class FrontendServerRequireStrategyProvider { MetadataProvider metadataProvider, String module) async => _addBasePath((await metadataProvider.moduleToSourceMap)[module] ?? ''); - String _serverPathForAppUri(String appUri) { + String? _serverPathForAppUri(String appUri) { if (appUri.startsWith('org-dartlang-app:')) { return _addBasePath(Uri.parse(appUri).path); } @@ -76,7 +84,7 @@ class FrontendServerRequireStrategyProvider { final modules = await metadataProvider.moduleToModulePath; final result = {}; for (var module in modules.keys) { - final modulePath = modules[module]; + final modulePath = modules[module]!; result[module] = ModuleInfo( // TODO: Save locations of full kernel files in ddc metadata. // Issue: https://github.com/dart-lang/sdk/issues/43684 diff --git a/dwds/lib/src/loaders/legacy.dart b/dwds/lib/src/loaders/legacy.dart index f910711b7..02e01487e 100644 --- a/dwds/lib/src/loaders/legacy.dart +++ b/dwds/lib/src/loaders/legacy.dart @@ -2,8 +2,6 @@ // 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. -// @dart = 2.9 - import 'package:shelf/shelf.dart'; import '../debugging/metadata/provider.dart'; @@ -61,7 +59,7 @@ class LegacyStrategy extends LoadStrategy { /// /// Will return `null` if the provided uri is not /// an app URI. - final String Function(String appUri) _serverPathForAppUri; + final String? Function(String appUri) _serverPathForAppUri; LegacyStrategy( this.reloadConfiguration, @@ -74,7 +72,7 @@ class LegacyStrategy extends LoadStrategy { ) : super(assetReader); @override - Handler get handler => (request) => null; + Handler get handler => (request) => Response.notFound(request.url); @override String get id => 'legacy'; @@ -121,5 +119,5 @@ class LegacyStrategy extends LoadStrategy { _sourceMapPathForModule(metadataProviderFor(entrypoint), module); @override - String serverPathForAppUri(String appUri) => _serverPathForAppUri(appUri); + String? serverPathForAppUri(String appUri) => _serverPathForAppUri(appUri); } diff --git a/dwds/lib/src/loaders/require.dart b/dwds/lib/src/loaders/require.dart index 30413d7af..1bdfa469a 100644 --- a/dwds/lib/src/loaders/require.dart +++ b/dwds/lib/src/loaders/require.dart @@ -2,8 +2,6 @@ // 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. -// @dart = 2.9 - import 'dart:convert'; import 'package:path/path.dart' as p; @@ -20,7 +18,6 @@ import '../services/expression_compiler.dart'; /// https://localhost/base/index.html => /base /// https://localhost/base => /base String basePathForServerUri(String url) { - if (url == null) return null; final uri = Uri.parse(url); var base = uri.path.endsWith('.html') ? p.dirname(uri.path) : uri.path; if (base.isNotEmpty) { @@ -131,7 +128,7 @@ class RequireStrategy extends LoadStrategy { /// /// Will return `null` if the provided uri is not /// an app URI. - final String Function(String appUri) _serverPathForAppUri; + final String? Function(String appUri) _serverPathForAppUri; /// Returns a map from module id to module info. /// @@ -157,13 +154,14 @@ class RequireStrategy extends LoadStrategy { @override Handler get handler => (request) async { if (request.url.path.endsWith(_requireDigestsPath)) { + final entrypoint = request.url.queryParameters['entrypoint']; + if (entrypoint == null) return Response.notFound('${request.url}'); final metadataProvider = - metadataProviderFor(request.url.queryParameters['entrypoint']); - if (metadataProvider == null) return null; + metadataProviderFor(request.url.queryParameters['entrypoint']!); final digests = await _digestsProvider(metadataProvider); return Response.ok(json.encode(digests)); } - return null; + return Response.notFound('${request.url}'); }; @override @@ -296,7 +294,7 @@ if(!window.\$requireLoader) { } @override - String serverPathForAppUri(String appUri) => _serverPathForAppUri(appUri); + String? serverPathForAppUri(String appUri) => _serverPathForAppUri(appUri); @override Future> moduleInfoForEntrypoint(String entrypoint) => diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart index c98ec8d45..07ff471e5 100644 --- a/dwds/lib/src/loaders/strategy.dart +++ b/dwds/lib/src/loaders/strategy.dart @@ -2,24 +2,17 @@ // 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. -// @dart = 2.9 - import 'package:shelf/shelf.dart'; import '../debugging/metadata/provider.dart'; import '../readers/asset_reader.dart'; import '../services/expression_compiler.dart'; -LoadStrategy _globalLoadStrategy; +late LoadStrategy _globalLoadStrategy; set globalLoadStrategy(LoadStrategy strategy) => _globalLoadStrategy = strategy; -LoadStrategy get globalLoadStrategy { - if (_globalLoadStrategy == null) { - throw StateError('Global load strategy not set'); - } - return _globalLoadStrategy; -} +LoadStrategy get globalLoadStrategy => _globalLoadStrategy; abstract class LoadStrategy { final AssetReader _assetReader; @@ -126,12 +119,17 @@ abstract class LoadStrategy { /// /// Will return `null` if the provided uri is not /// an app URI. - String serverPathForAppUri(String appUri); + String? serverPathForAppUri(String appUri); /// Returns the [MetadataProvider] for the application located at the provided /// [entrypoint]. - MetadataProvider metadataProviderFor(String entrypoint) => - _providers[entrypoint]; + MetadataProvider metadataProviderFor(String entrypoint) { + if (_providers.containsKey(entrypoint)) { + return _providers[entrypoint]!; + } else { + throw StateError('No metadata provider for $entrypoint'); + } + } /// Initializes a [MetadataProvider] for the application located at the /// provided [entrypoint]. diff --git a/dwds/test/evaluate_common.dart b/dwds/test/evaluate_common.dart index ad452f692..3f861f646 100644 --- a/dwds/test/evaluate_common.dart +++ b/dwds/test/evaluate_common.dart @@ -71,6 +71,7 @@ void testAll({ setUpAll(() async { setCurrentLogWriter(debug: debug); await context.setUp( + compilationMode: compilationMode, enableExpressionEvaluation: true, verboseCompiler: debug, basePath: basePath, diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index 198bb46a4..e9be74240 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -277,6 +277,7 @@ class TestContext { assetReader = webRunner.devFS.assetServer; assetHandler = webRunner.devFS.assetServer.handleRequest; + _logger.warning('BasePath: $basePath'); requireStrategy = FrontendServerRequireStrategyProvider( reloadConfiguration, assetReader, () async => {}, basePath) .strategy; From 90ff5caed6954f2ac95e935b880f4942745247a6 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 17 Jun 2022 15:30:50 -0700 Subject: [PATCH 3/8] Add format errors --- .../debugging/metadata/module_metadata.dart | 23 +++++++++++-------- .../src/loaders/frontend_server_require.dart | 6 ++--- dwds/test/fixtures/context.dart | 1 - 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/dwds/lib/src/debugging/metadata/module_metadata.dart b/dwds/lib/src/debugging/metadata/module_metadata.dart index 9f4ae4c30..048d781f9 100644 --- a/dwds/lib/src/debugging/metadata/module_metadata.dart +++ b/dwds/lib/src/debugging/metadata/module_metadata.dart @@ -175,17 +175,22 @@ class ModuleMetadata { } } -// TODO: format errors! -T _readRequiredField(Map json, String field) => - json[field]! as T; +T _readRequiredField(Map json, String field) { + if (!json.containsKey(field)) { + throw FormatException('Required field $field is not set in $json'); + } + return json[field]! as T; +} T? _readOptionalField(Map json, String field) => json[field] as T?; -List _readRequiredList(Map json, String field) => - List.castFrom(json[field] as List); +List _readRequiredList(Map json, String field) { + final list = _readRequiredField>(json, field); + return List.castFrom(list); +} -List? _readOptionalList(Map json, String field) => - json.containsKey(field) - ? List.castFrom(json[field] as List) - : null; +List? _readOptionalList(Map json, String field) { + final list = _readOptionalField>(json, field); + return list == null ? null : List.castFrom(list); +} diff --git a/dwds/lib/src/loaders/frontend_server_require.dart b/dwds/lib/src/loaders/frontend_server_require.dart index 680870757..17ff20782 100644 --- a/dwds/lib/src/loaders/frontend_server_require.dart +++ b/dwds/lib/src/loaders/frontend_server_require.dart @@ -56,10 +56,8 @@ class FrontendServerRequireStrategyProvider { MetadataProvider metadataProvider, String serverPath) async { final modulePathToModule = await metadataProvider.modulePathToModule; final relativeServerPath = _removeBasePath(serverPath); - if (relativeServerPath == null || - !modulePathToModule.containsKey(relativeServerPath)) { - _logger.warning( - 'No module found for server path: $serverPath. Server path is not under $_basePath.'); + if (!modulePathToModule.containsKey(relativeServerPath)) { + _logger.warning('No module found for server path: $serverPath.'); return ''; } return modulePathToModule[relativeServerPath]!; diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index ca27c9a36..1d1a26b68 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -291,7 +291,6 @@ class TestContext { assetReader = webRunner.devFS.assetServer; assetHandler = webRunner.devFS.assetServer.handleRequest; - _logger.warning('BasePath: $basePath'); requireStrategy = FrontendServerRequireStrategyProvider( reloadConfiguration, assetReader, () async => {}, basePath) .strategy; From 171b9a9e881e46b00a1aad62275b0577ff377a56 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 17 Jun 2022 15:46:45 -0700 Subject: [PATCH 4/8] Removed unnecessary 'late' from metadata fields --- .../debugging/metadata/module_metadata.dart | 37 +++++++++---------- dwds/lib/src/handlers/injector.dart | 2 +- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/dwds/lib/src/debugging/metadata/module_metadata.dart b/dwds/lib/src/debugging/metadata/module_metadata.dart index 048d781f9..5074c20f8 100644 --- a/dwds/lib/src/debugging/metadata/module_metadata.dart +++ b/dwds/lib/src/debugging/metadata/module_metadata.dart @@ -60,25 +60,24 @@ class ModuleMetadataVersion { /// See: https://goto.google.com/dart-web-debugger-metadata class LibraryMetadata { /// Library name as defined in pubspec.yaml - late final String name; + final String name; /// Library importUri /// /// Example package:path/path.dart - late final String importUri; + final String importUri; /// All file uris from the library /// /// Can be relative paths to the directory of the fileUri - late final List partUris; + final List partUris; LibraryMetadata(this.name, this.importUri, this.partUris); - LibraryMetadata.fromJson(Map json) { - name = _readRequiredField(json, 'name'); - importUri = _readRequiredField(json, 'importUri'); + LibraryMetadata.fromJson(Map json) : + name = _readRequiredField(json, 'name'), + importUri = _readRequiredField(json, 'importUri'), partUris = _readOptionalList(json, 'partUris') ?? []; - } Map toJson() { return { @@ -102,22 +101,22 @@ class ModuleMetadata { /// /// Used as a name of the js module created by the compiler and /// as key to store and load modules in the debugger and the browser - late final String name; + final String name; /// Name of the function enclosing the module /// /// Used by debugger to determine the top dart scope - late final String closureName; + final String closureName; /// Source map uri - late final String sourceMapUri; + final String sourceMapUri; /// Module uri - late final String moduleUri; + final String moduleUri; /// True if the module corresponding to this metadata was compiled with sound /// null safety enabled. - late final bool soundNullSafety; + final bool soundNullSafety; final Map libraries = {}; @@ -142,13 +141,13 @@ class ModuleMetadata { } } - ModuleMetadata.fromJson(Map json) { - version = _readRequiredField(json, 'version'); - name = _readRequiredField(json, 'name'); - closureName = _readRequiredField(json, 'closureName'); - sourceMapUri = _readRequiredField(json, 'sourceMapUri'); - moduleUri = _readRequiredField(json, 'moduleUri'); - soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false; + ModuleMetadata.fromJson(Map json): + version = _readRequiredField(json, 'version'), + name = _readRequiredField(json, 'name'), + closureName = _readRequiredField(json, 'closureName'), + sourceMapUri = _readRequiredField(json, 'sourceMapUri'), + moduleUri = _readRequiredField(json, 'moduleUri'), + soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false { if (!ModuleMetadataVersion.current.isCompatibleWith(version) && !ModuleMetadataVersion.previous.isCompatibleWith(version)) { throw Exception('Unsupported metadata version $version. ' diff --git a/dwds/lib/src/handlers/injector.dart b/dwds/lib/src/handlers/injector.dart index d0089a41f..ee740a2bb 100644 --- a/dwds/lib/src/handlers/injector.dart +++ b/dwds/lib/src/handlers/injector.dart @@ -123,7 +123,7 @@ class DwdsInjector { return response.change(body: body, headers: newHeaders); } else { final loadResponse = await _loadStrategy.handler(request); - if (loadResponse != null && loadResponse.statusCode != 404) { + if (loadResponse.statusCode != HttpStatus.notFound) { return loadResponse; } return innerHandler(request); From 3a93741b2fd2a80cca06594ee92fb519ca1b2405 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 17 Jun 2022 15:49:01 -0700 Subject: [PATCH 5/8] Format --- .../debugging/metadata/module_metadata.dart | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/dwds/lib/src/debugging/metadata/module_metadata.dart b/dwds/lib/src/debugging/metadata/module_metadata.dart index 5074c20f8..fffd5916e 100644 --- a/dwds/lib/src/debugging/metadata/module_metadata.dart +++ b/dwds/lib/src/debugging/metadata/module_metadata.dart @@ -74,10 +74,10 @@ class LibraryMetadata { LibraryMetadata(this.name, this.importUri, this.partUris); - LibraryMetadata.fromJson(Map json) : - name = _readRequiredField(json, 'name'), - importUri = _readRequiredField(json, 'importUri'), - partUris = _readOptionalList(json, 'partUris') ?? []; + LibraryMetadata.fromJson(Map json) + : name = _readRequiredField(json, 'name'), + importUri = _readRequiredField(json, 'importUri'), + partUris = _readOptionalList(json, 'partUris') ?? []; Map toJson() { return { @@ -141,13 +141,13 @@ class ModuleMetadata { } } - ModuleMetadata.fromJson(Map json): - version = _readRequiredField(json, 'version'), - name = _readRequiredField(json, 'name'), - closureName = _readRequiredField(json, 'closureName'), - sourceMapUri = _readRequiredField(json, 'sourceMapUri'), - moduleUri = _readRequiredField(json, 'moduleUri'), - soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false { + ModuleMetadata.fromJson(Map json) + : version = _readRequiredField(json, 'version'), + name = _readRequiredField(json, 'name'), + closureName = _readRequiredField(json, 'closureName'), + sourceMapUri = _readRequiredField(json, 'sourceMapUri'), + moduleUri = _readRequiredField(json, 'moduleUri'), + soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false { if (!ModuleMetadataVersion.current.isCompatibleWith(version) && !ModuleMetadataVersion.previous.isCompatibleWith(version)) { throw Exception('Unsupported metadata version $version. ' From 3e84a5846a527e586d587fbe23a2a94c39b7421b Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 17 Jun 2022 17:21:47 -0700 Subject: [PATCH 6/8] Fix failing test --- dwds/lib/src/handlers/injector.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/handlers/injector.dart b/dwds/lib/src/handlers/injector.dart index ee740a2bb..4016eaf49 100644 --- a/dwds/lib/src/handlers/injector.dart +++ b/dwds/lib/src/handlers/injector.dart @@ -123,7 +123,8 @@ class DwdsInjector { return response.change(body: body, headers: newHeaders); } else { final loadResponse = await _loadStrategy.handler(request); - if (loadResponse.statusCode != HttpStatus.notFound) { + if (loadResponse != null && + loadResponse.statusCode != HttpStatus.notFound) { return loadResponse; } return innerHandler(request); From d8d5e7220aecce06f4be008fb98adf88043468be Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Tue, 21 Jun 2022 10:11:38 -0700 Subject: [PATCH 7/8] Addressed CR comments --- dwds/lib/src/loaders/build_runner_require.dart | 3 +-- dwds/lib/src/loaders/frontend_server_require.dart | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/dwds/lib/src/loaders/build_runner_require.dart b/dwds/lib/src/loaders/build_runner_require.dart index dbdda0a08..844cc87e3 100644 --- a/dwds/lib/src/loaders/build_runner_require.dart +++ b/dwds/lib/src/loaders/build_runner_require.dart @@ -81,8 +81,7 @@ class BuildRunnerRequireStrategyProvider { return e.value; } } - _logger.warning('No module for server path $serverPath'); - return ''; + throw StateError('No module found for server path $serverPath'); } Future _serverPathForModule( diff --git a/dwds/lib/src/loaders/frontend_server_require.dart b/dwds/lib/src/loaders/frontend_server_require.dart index 17ff20782..1b358b2b0 100644 --- a/dwds/lib/src/loaders/frontend_server_require.dart +++ b/dwds/lib/src/loaders/frontend_server_require.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; import '../debugging/metadata/provider.dart'; @@ -13,7 +12,6 @@ import 'require.dart'; /// Provides a [RequireStrategy] suitable for use with Frontend Server. class FrontendServerRequireStrategyProvider { - final _logger = Logger('FrontendServerRequireStrategyProvider'); final ReloadConfiguration _configuration; final AssetReader _assetReader; final Future> Function() _digestsProvider; @@ -57,8 +55,7 @@ class FrontendServerRequireStrategyProvider { final modulePathToModule = await metadataProvider.modulePathToModule; final relativeServerPath = _removeBasePath(serverPath); if (!modulePathToModule.containsKey(relativeServerPath)) { - _logger.warning('No module found for server path: $serverPath.'); - return ''; + throw StateError('No module found for server path $serverPath'); } return modulePathToModule[relativeServerPath]!; } From 7896af1eb4586ab1db7d773408d0732572e23c18 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Tue, 21 Jun 2022 14:06:29 -0700 Subject: [PATCH 8/8] Relax some require API to allow nulls, fix test failures --- dwds/lib/src/loaders/build_runner_require.dart | 4 ++-- dwds/lib/src/loaders/frontend_server_require.dart | 7 ++----- dwds/lib/src/loaders/require.dart | 4 ++-- dwds/lib/src/loaders/strategy.dart | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/dwds/lib/src/loaders/build_runner_require.dart b/dwds/lib/src/loaders/build_runner_require.dart index 844cc87e3..b679e21bf 100644 --- a/dwds/lib/src/loaders/build_runner_require.dart +++ b/dwds/lib/src/loaders/build_runner_require.dart @@ -72,7 +72,7 @@ class BuildRunnerRequireStrategyProvider { (await metadataProvider.moduleToModulePath).map((key, value) => MapEntry(key, stripTopLevelDirectory(removeJsExtension(value)))); - Future _moduleForServerPath( + Future _moduleForServerPath( MetadataProvider metadataProvider, String serverPath) async { final modulePathToModule = await metadataProvider.modulePathToModule; final relativePath = relativizePath(serverPath); @@ -81,7 +81,7 @@ class BuildRunnerRequireStrategyProvider { return e.value; } } - throw StateError('No module found for server path $serverPath'); + return null; } Future _serverPathForModule( diff --git a/dwds/lib/src/loaders/frontend_server_require.dart b/dwds/lib/src/loaders/frontend_server_require.dart index 1b358b2b0..4a63ea026 100644 --- a/dwds/lib/src/loaders/frontend_server_require.dart +++ b/dwds/lib/src/loaders/frontend_server_require.dart @@ -50,14 +50,11 @@ class FrontendServerRequireStrategyProvider { (await metadataProvider.moduleToModulePath).map((key, value) => MapEntry(key, relativizePath(removeJsExtension(value)))); - Future _moduleForServerPath( + Future _moduleForServerPath( MetadataProvider metadataProvider, String serverPath) async { final modulePathToModule = await metadataProvider.modulePathToModule; final relativeServerPath = _removeBasePath(serverPath); - if (!modulePathToModule.containsKey(relativeServerPath)) { - throw StateError('No module found for server path $serverPath'); - } - return modulePathToModule[relativeServerPath]!; + return modulePathToModule[relativeServerPath]; } Future _serverPathForModule( diff --git a/dwds/lib/src/loaders/require.dart b/dwds/lib/src/loaders/require.dart index 87cfa7809..b9ba9b6fd 100644 --- a/dwds/lib/src/loaders/require.dart +++ b/dwds/lib/src/loaders/require.dart @@ -96,7 +96,7 @@ class RequireStrategy extends LoadStrategy { /// /// /packages/path/path.ddc.js -> packages/path/path /// - final Future Function(MetadataProvider provider, String sourcePath) + final Future Function(MetadataProvider provider, String sourcePath) _moduleForServerPath; /// Returns the server path for the provided module. @@ -273,7 +273,7 @@ if(!window.\$requireLoader) { } @override - Future moduleForServerPath(String entrypoint, String serverPath) { + Future moduleForServerPath(String entrypoint, String serverPath) { final metadataProvider = metadataProviderFor(entrypoint); return _moduleForServerPath(metadataProvider, serverPath); } diff --git a/dwds/lib/src/loaders/strategy.dart b/dwds/lib/src/loaders/strategy.dart index 07ff471e5..ece82bf41 100644 --- a/dwds/lib/src/loaders/strategy.dart +++ b/dwds/lib/src/loaders/strategy.dart @@ -85,7 +85,7 @@ abstract class LoadStrategy { /// /// /packages/path/path.ddc.js -> packages/path/path /// - Future moduleForServerPath(String entrypoint, String serverPath); + Future moduleForServerPath(String entrypoint, String serverPath); /// Returns the server path for the provided module. ///