Skip to content

Commit 0736777

Browse files
authored
Pass the package config directly to the load strategy instead of depending on an app entrypoint (#2203)
1 parent 0044d75 commit 0736777

File tree

6 files changed

+19
-57
lines changed

6 files changed

+19
-57
lines changed

dwds/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
**Breaking changes**
44

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

88
## 20.0.1

dwds/lib/src/loaders/legacy.dart

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,6 @@ class LegacyStrategy extends LoadStrategy {
6666
/// an app URI.
6767
final String? Function(String appUri) _serverPathForAppUri;
6868

69-
/// Returns the absolute path to the app's package config, determined by the
70-
/// app's [entrypoint] path.
71-
///
72-
/// Example:
73-
///
74-
/// main_module.bootstrap.js
75-
/// -> /Users/john_doe/my_dart_app/.dart_tool/package_config.json
76-
///
77-
final String? Function(String entrypoint) _packageConfigLocator;
78-
7969
/// Returns the relative path in google3, determined by the [absolutePath].
8070
///
8171
/// Returns `null` if not a google3 app.
@@ -92,9 +82,9 @@ class LegacyStrategy extends LoadStrategy {
9282
this._moduleInfoForProvider,
9383
AssetReader assetReader,
9484
this._appEntrypoint,
95-
this._packageConfigLocator,
9685
this._g3RelativePath,
97-
) : super(assetReader);
86+
String? packageConfigPath,
87+
) : super(assetReader, packageConfigPath: packageConfigPath);
9888

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

143-
@override
144-
String? packageConfigLocator(String entrypoint) =>
145-
_packageConfigLocator(entrypoint);
146-
147133
@override
148134
String? g3RelativePath(String absolutePath) => _g3RelativePath(absolutePath);
149135
}

dwds/lib/src/loaders/require.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -284,9 +284,6 @@ if(!window.\$requireLoader) {
284284
Future<Map<String, ModuleInfo>> moduleInfoForEntrypoint(String entrypoint) =>
285285
_moduleInfoForProvider(metadataProviderFor(entrypoint));
286286

287-
@override
288-
String? packageConfigLocator(String entrypoint) => null;
289-
290287
@override
291288
String? g3RelativePath(String absolutePath) => null;
292289
}

dwds/lib/src/loaders/strategy.dart

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@ import 'package:shelf/shelf.dart';
1111

1212
abstract class LoadStrategy {
1313
final AssetReader _assetReader;
14+
final String? _packageConfigPath;
1415
final _providers = <String, MetadataProvider>{};
15-
String? _packageConfigPath;
1616

17-
LoadStrategy(this._assetReader);
17+
LoadStrategy(
18+
this._assetReader, {
19+
String? packageConfigPath,
20+
}) : _packageConfigPath = packageConfigPath;
1821

1922
/// The ID for this strategy.
2023
///
@@ -100,28 +103,17 @@ abstract class LoadStrategy {
100103
/// an app URI.
101104
String? serverPathForAppUri(String appUri);
102105

103-
/// Returns the absolute path to the app's package config, determined by the
104-
/// app's [entrypoint] path.
105-
///
106-
/// Example:
107-
///
108-
/// main_module.bootstrap.js
109-
/// -> /Users/john_doe/my_dart_app/.dart_tool/package_config.json
110-
///
111-
String? packageConfigLocator(String entrypoint);
112-
113106
/// Returns the relative path in google3, determined by the [absolutePath].
114107
///
115108
/// Returns `null` if not a google3 app.
116109
String? g3RelativePath(String absolutePath);
117110

118-
/// The absolute path to the app's package config, or null if not provided by
119-
/// [packageConfigLocator].
111+
/// The absolute path to the app's package configuration.
120112
String get packageConfigPath {
121113
return _packageConfigPath ?? _defaultPackageConfigPath;
122114
}
123115

124-
/// The default package config path, if none is provided by the load strategy.
116+
/// The default package config path if none is provided.
125117
String get _defaultPackageConfigPath => p.join(
126118
DartUri.currentDirectory,
127119
'.dart_tool',
@@ -142,7 +134,6 @@ abstract class LoadStrategy {
142134
/// provided [entrypoint].
143135
void trackEntrypoint(String entrypoint) {
144136
final metadataProvider = MetadataProvider(entrypoint, _assetReader);
145-
_packageConfigPath = packageConfigLocator(entrypoint);
146137
_providers[metadataProvider.entrypoint] = metadataProvider;
147138
}
148139
}

dwds/test/fixtures/fakes.dart

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,9 @@ class FakeExecutionContext extends ExecutionContext {
314314

315315
class FakeStrategy extends LoadStrategy {
316316
FakeStrategy(
317-
AssetReader assetReader,
318-
) : super(assetReader);
317+
AssetReader assetReader, {
318+
String? packageConfigPath,
319+
}) : super(assetReader, packageConfigPath: packageConfigPath);
319320

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

344-
@override
345-
String? packageConfigLocator(String entrypoint) => null;
346-
347345
@override
348346
String? g3RelativePath(String absolutePath) => null;
349347

dwds/test/load_strategy_test.dart

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
@TestOn('vm')
66
@Timeout(Duration(minutes: 1))
77

8-
import 'package:dwds/asset_reader.dart';
98
import 'package:path/path.dart' as p;
109
import 'package:test/test.dart';
1110
import 'package:test_common/test_sdk_configuration.dart';
@@ -14,16 +13,6 @@ import 'fixtures/context.dart';
1413
import 'fixtures/fakes.dart';
1514
import 'fixtures/project.dart';
1615

17-
class LoadStrategyCustomPackageConfig extends FakeStrategy {
18-
LoadStrategyCustomPackageConfig(
19-
AssetReader assetReader,
20-
) : super(assetReader);
21-
22-
@override
23-
String? packageConfigLocator(String entrypoint) =>
24-
'$entrypoint/custom/package_config/path';
25-
}
26-
2716
void main() {
2817
final provider = TestSdkConfigurationProvider();
2918
tearDownAll(provider.dispose);
@@ -45,22 +34,23 @@ void main() {
4534
final strategy = FakeStrategy(FakeAssetReader());
4635

4736
test('defaults to "./dart_tool/packageconfig.json"', () {
48-
strategy.trackEntrypoint('my_app/entrypoint');
4937
expect(
5038
p.split(strategy.packageConfigPath).join('/'),
5139
endsWith('_testSound/.dart_tool/package_config.json'),
5240
);
5341
});
5442
});
5543

56-
group('When the packageConfigLocator specifies a package config path', () {
57-
final strategy = LoadStrategyCustomPackageConfig(FakeAssetReader());
44+
group('When a custom package config path is specified', () {
45+
final strategy = FakeStrategy(
46+
FakeAssetReader(),
47+
packageConfigPath: 'custom/package_config/path',
48+
);
5849

5950
test('uses the specified package config path', () {
60-
strategy.trackEntrypoint('my_app/entrypoint');
6151
expect(
6252
strategy.packageConfigPath,
63-
equals('my_app/entrypoint/custom/package_config/path'),
53+
equals('custom/package_config/path'),
6454
);
6555
});
6656
});

0 commit comments

Comments
 (0)