Skip to content

Commit

Permalink
Pass the package config directly to the load strategy instead of depe…
Browse files Browse the repository at this point in the history
…nding on an app entrypoint (#2203)
  • Loading branch information
elliette authored Aug 31, 2023
1 parent 0044d75 commit 0736777
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 57 deletions.
2 changes: 1 addition & 1 deletion dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

**Breaking changes**

- Allow clients to specify where to find the package config. - [#2199](https://github.com/dart-lang/webdev/pull/2199).
- Allow clients to specify where to find the package config. - [#2203](https://github.com/dart-lang/webdev/pull/2203).
- Allow clients to specify a way to convert absolute paths to g3-relative paths. - [#2200](https://github.com/dart-lang/webdev/pull/2200)

## 20.0.1
Expand Down
18 changes: 2 additions & 16 deletions dwds/lib/src/loaders/legacy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@ class LegacyStrategy extends LoadStrategy {
/// an app URI.
final String? Function(String appUri) _serverPathForAppUri;

/// Returns the absolute path to the app's package config, determined by the
/// app's [entrypoint] path.
///
/// Example:
///
/// main_module.bootstrap.js
/// -> /Users/john_doe/my_dart_app/.dart_tool/package_config.json
///
final String? Function(String entrypoint) _packageConfigLocator;

/// Returns the relative path in google3, determined by the [absolutePath].
///
/// Returns `null` if not a google3 app.
Expand All @@ -92,9 +82,9 @@ class LegacyStrategy extends LoadStrategy {
this._moduleInfoForProvider,
AssetReader assetReader,
this._appEntrypoint,
this._packageConfigLocator,
this._g3RelativePath,
) : super(assetReader);
String? packageConfigPath,
) : super(assetReader, packageConfigPath: packageConfigPath);

@override
Handler get handler => (request) => Response.notFound(request.url.toString());
Expand Down Expand Up @@ -140,10 +130,6 @@ class LegacyStrategy extends LoadStrategy {
@override
Uri? get appEntrypoint => _appEntrypoint;

@override
String? packageConfigLocator(String entrypoint) =>
_packageConfigLocator(entrypoint);

@override
String? g3RelativePath(String absolutePath) => _g3RelativePath(absolutePath);
}
3 changes: 0 additions & 3 deletions dwds/lib/src/loaders/require.dart
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,6 @@ if(!window.\$requireLoader) {
Future<Map<String, ModuleInfo>> moduleInfoForEntrypoint(String entrypoint) =>
_moduleInfoForProvider(metadataProviderFor(entrypoint));

@override
String? packageConfigLocator(String entrypoint) => null;

@override
String? g3RelativePath(String absolutePath) => null;
}
23 changes: 7 additions & 16 deletions dwds/lib/src/loaders/strategy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ import 'package:shelf/shelf.dart';

abstract class LoadStrategy {
final AssetReader _assetReader;
final String? _packageConfigPath;
final _providers = <String, MetadataProvider>{};
String? _packageConfigPath;

LoadStrategy(this._assetReader);
LoadStrategy(
this._assetReader, {
String? packageConfigPath,
}) : _packageConfigPath = packageConfigPath;

/// The ID for this strategy.
///
Expand Down Expand Up @@ -100,28 +103,17 @@ abstract class LoadStrategy {
/// an app URI.
String? serverPathForAppUri(String appUri);

/// Returns the absolute path to the app's package config, determined by the
/// app's [entrypoint] path.
///
/// Example:
///
/// main_module.bootstrap.js
/// -> /Users/john_doe/my_dart_app/.dart_tool/package_config.json
///
String? packageConfigLocator(String entrypoint);

/// Returns the relative path in google3, determined by the [absolutePath].
///
/// Returns `null` if not a google3 app.
String? g3RelativePath(String absolutePath);

/// The absolute path to the app's package config, or null if not provided by
/// [packageConfigLocator].
/// The absolute path to the app's package configuration.
String get packageConfigPath {
return _packageConfigPath ?? _defaultPackageConfigPath;
}

/// The default package config path, if none is provided by the load strategy.
/// The default package config path if none is provided.
String get _defaultPackageConfigPath => p.join(
DartUri.currentDirectory,
'.dart_tool',
Expand All @@ -142,7 +134,6 @@ abstract class LoadStrategy {
/// provided [entrypoint].
void trackEntrypoint(String entrypoint) {
final metadataProvider = MetadataProvider(entrypoint, _assetReader);
_packageConfigPath = packageConfigLocator(entrypoint);
_providers[metadataProvider.entrypoint] = metadataProvider;
}
}
Expand Down
8 changes: 3 additions & 5 deletions dwds/test/fixtures/fakes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,9 @@ class FakeExecutionContext extends ExecutionContext {

class FakeStrategy extends LoadStrategy {
FakeStrategy(
AssetReader assetReader,
) : super(assetReader);
AssetReader assetReader, {
String? packageConfigPath,
}) : super(assetReader, packageConfigPath: packageConfigPath);

@override
Future<String> bootstrapFor(String entrypoint) async => 'dummy_bootstrap';
Expand All @@ -341,9 +342,6 @@ class FakeStrategy extends LoadStrategy {
@override
Uri? get appEntrypoint => Uri.parse('package:myapp/main.dart');

@override
String? packageConfigLocator(String entrypoint) => null;

@override
String? g3RelativePath(String absolutePath) => null;

Expand Down
22 changes: 6 additions & 16 deletions dwds/test/load_strategy_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
@TestOn('vm')
@Timeout(Duration(minutes: 1))

import 'package:dwds/asset_reader.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_common/test_sdk_configuration.dart';
Expand All @@ -14,16 +13,6 @@ import 'fixtures/context.dart';
import 'fixtures/fakes.dart';
import 'fixtures/project.dart';

class LoadStrategyCustomPackageConfig extends FakeStrategy {
LoadStrategyCustomPackageConfig(
AssetReader assetReader,
) : super(assetReader);

@override
String? packageConfigLocator(String entrypoint) =>
'$entrypoint/custom/package_config/path';
}

void main() {
final provider = TestSdkConfigurationProvider();
tearDownAll(provider.dispose);
Expand All @@ -45,22 +34,23 @@ void main() {
final strategy = FakeStrategy(FakeAssetReader());

test('defaults to "./dart_tool/packageconfig.json"', () {
strategy.trackEntrypoint('my_app/entrypoint');
expect(
p.split(strategy.packageConfigPath).join('/'),
endsWith('_testSound/.dart_tool/package_config.json'),
);
});
});

group('When the packageConfigLocator specifies a package config path', () {
final strategy = LoadStrategyCustomPackageConfig(FakeAssetReader());
group('When a custom package config path is specified', () {
final strategy = FakeStrategy(
FakeAssetReader(),
packageConfigPath: 'custom/package_config/path',
);

test('uses the specified package config path', () {
strategy.trackEntrypoint('my_app/entrypoint');
expect(
strategy.packageConfigPath,
equals('my_app/entrypoint/custom/package_config/path'),
equals('custom/package_config/path'),
);
});
});
Expand Down

0 comments on commit 0736777

Please sign in to comment.