Skip to content

Commit 294a50f

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
[frontend_server] Soft-deprecate moduleName for compileExpressionToJS
#58265 The DDC library bundle format does not give names to modules. Therefore the frontend server should try and find the compiler associated with the library and not the module to be consistent. This then means that moduleName becomes entirely unused, and therefore we can soft-deprecate it. Change-Id: I241c63a346d046405599384409a373ab12be2654 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396102 Commit-Queue: Srujan Gaddam <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 046c968 commit 294a50f

File tree

4 files changed

+26
-57
lines changed

4 files changed

+26
-57
lines changed

‎pkg/frontend_server/lib/frontend_server.dart‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ abstract class CompilerInterface {
348348
bool isStatic);
349349

350350
/// Compiles [expression] in library [libraryUri] and file [scriptUri]
351-
/// at [line]:[column] to JavaScript in [moduleName].
351+
/// at [line]:[column] to JavaScript.
352352
///
353353
/// [libraryUri] and [scriptUri] can be the same, but if for instance
354354
/// evaluating expressions in a part file the [libraryUri] will be the uri of
@@ -363,7 +363,6 @@ abstract class CompilerInterface {
363363
/// expression.
364364
///
365365
/// Example values of parameters:
366-
/// [moduleName] is of the form '/packages/hello_world_main.dart'
367366
/// [jsFrameValues] is a map from js variable name to its primitive value
368367
/// or another variable name, for example
369368
/// { 'x': '1', 'y': 'y', 'o': 'null' }
@@ -378,7 +377,6 @@ abstract class CompilerInterface {
378377
int column,
379378
Map<String, String> jsModules,
380379
Map<String, String> jsFrameValues,
381-
String moduleName,
382380
String expression);
383381

384382
/// Communicates an error [msg] to the client.
@@ -1102,10 +1100,10 @@ class FrontendCompiler implements CompilerInterface {
11021100
}
11031101
}
11041102

1105-
/// Program compilers per module.
1103+
/// Mapping of libraries to their program compiler.
11061104
///
1107-
/// Produced during initial compilation of the module to JavaScript,
1108-
/// cached to be used for expression compilation in [compileExpressionToJs].
1105+
/// Produced during initial compilation of the component to JavaScript, cached
1106+
/// to be used for expression compilation in [compileExpressionToJs].
11091107
final Map<String, Compiler> cachedProgramCompilers = {};
11101108

11111109
@override
@@ -1116,7 +1114,6 @@ class FrontendCompiler implements CompilerInterface {
11161114
int column,
11171115
Map<String, String> jsModules,
11181116
Map<String, String> jsFrameValues,
1119-
String moduleName,
11201117
String expression) async {
11211118
_generator.accept();
11221119
errors.clear();
@@ -1125,18 +1122,17 @@ class FrontendCompiler implements CompilerInterface {
11251122
reportError('JavaScript bundler is null');
11261123
return;
11271124
}
1128-
if (!cachedProgramCompilers.containsKey(moduleName)) {
1129-
reportError('Cannot find kernel2js compiler for $moduleName.');
1125+
if (!cachedProgramCompilers.containsKey(libraryUri)) {
1126+
reportError('Cannot find kernel2js compiler for $libraryUri.');
11301127
return;
11311128
}
1132-
11331129
final String boundaryKey = generateV4UUID();
11341130
_outputStream.writeln('result $boundaryKey');
11351131

11361132
_processedOptions.ticker
1137-
.logMs('Compiling expression to JavaScript in $moduleName');
1133+
.logMs('Compiling expression to JavaScript in $libraryUri');
11381134

1139-
final Compiler kernel2jsCompiler = cachedProgramCompilers[moduleName]!;
1135+
final Compiler kernel2jsCompiler = cachedProgramCompilers[libraryUri]!;
11401136
IncrementalCompilerResult compilerResult = _generator.lastKnownGoodResult!;
11411137
Component component = compilerResult.component;
11421138
component.computeCanonicalNames();
@@ -1358,7 +1354,6 @@ class _CompileExpressionToJsRequest {
13581354
late int column;
13591355
Map<String, String> jsModules = <String, String>{};
13601356
Map<String, String> jsFrameValues = <String, String>{};
1361-
late String moduleName;
13621357
late String expression;
13631358
}
13641359

@@ -1582,7 +1577,9 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
15821577
}
15831578
break;
15841579
case _State.COMPILE_EXPRESSION_TO_JS_MODULENAME:
1585-
compileExpressionToJsRequest.moduleName = string;
1580+
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
1581+
// soft-deprecated, so we still need to consume the string value, but it
1582+
// is unused.
15861583
state = _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION;
15871584
break;
15881585
case _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION:
@@ -1594,7 +1591,6 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
15941591
compileExpressionToJsRequest.column,
15951592
compileExpressionToJsRequest.jsModules,
15961593
compileExpressionToJsRequest.jsFrameValues,
1597-
compileExpressionToJsRequest.moduleName,
15981594
compileExpressionToJsRequest.expression);
15991595
state = _State.READY_FOR_INSTRUCTION;
16001596
break;
@@ -1732,12 +1728,14 @@ Future<void> processJsonInput(
17321728
int column = getValue<int>("column") ?? -1;
17331729
Map<String, String> jsModules = getMap("jsModules") ?? {};
17341730
Map<String, String> jsFrameValues = getMap("jsFrameValues") ?? {};
1735-
String moduleName = getValue<String>("moduleName") ?? "";
17361731

17371732
if (errorMessages.isNotEmpty) {
17381733
compiler.reportError("Errors: $errorMessages.");
17391734
return;
17401735
}
1736+
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
1737+
// soft-deprecated, and therefore is unused.
1738+
unusedKeys.remove("moduleName");
17411739
if (unusedKeys.isNotEmpty) {
17421740
compiler.reportError("Errors: Send over unused data: $unusedKeys.");
17431741
}
@@ -1749,7 +1747,6 @@ Future<void> processJsonInput(
17491747
column,
17501748
jsModules,
17511749
jsFrameValues,
1752-
moduleName,
17531750
expression,
17541751
);
17551752
} else {

‎pkg/frontend_server/lib/src/javascript_bundle.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ class IncrementalJavaScriptBundler {
231231
final Program jsModule = compiler.emitModule(summaryComponent);
232232

233233
// Save program compiler to reuse for expression evaluation.
234-
kernel2JsCompilers[moduleName] = compiler;
234+
kernel2JsCompilers[library.importUri.toString()] = compiler;
235235

236236
String? sourceMapBase;
237237
if (moduleUri.isScheme('package')) {

‎pkg/frontend_server/test/frontend_server_test.dart‎

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,6 @@ extension type Foo(int value) {
795795
}
796796
''');
797797
String library = 'package:hello/foo.dart';
798-
String module = 'packages/hello/foo.dart';
799798

800799
File dillFile = new File('${tempDir.path}/foo.dart.dill');
801800
File sourceFile = new File('${dillFile.path}.sources');
@@ -835,7 +834,6 @@ extension type Foo(int value) {
835834
libraryUri: library,
836835
line: 5,
837836
column: 3,
838-
moduleName: module,
839837
scriptUri: file.uri,
840838
jsFrameValues: {"f": "42"});
841839
count += 1;
@@ -850,7 +848,6 @@ extension type Foo(int value) {
850848
libraryUri: library,
851849
line: 11,
852850
column: 5,
853-
moduleName: module,
854851
scriptUri: file.uri,
855852
jsFrameValues: {r"$this": "42"});
856853
count += 1;
@@ -891,7 +888,6 @@ extension type Foo(int value) {
891888
];
892889

893890
String library = 'package:hello/foo.dart';
894-
String module = 'packages/hello/foo.dart';
895891

896892
FrontendServer frontendServer = new FrontendServer();
897893
Future<int> result = frontendServer.open(args);
@@ -918,11 +914,7 @@ extension type Foo(int value) {
918914
expect(outputFile.lengthSync(), isPositive);
919915

920916
frontendServer.compileExpressionToJs(
921-
expression: '',
922-
libraryUri: library,
923-
line: 1,
924-
column: 1,
925-
moduleName: module);
917+
expression: '', libraryUri: library, line: 1, column: 1);
926918
count += 1;
927919
} else if (count == 2) {
928920
// Third request is to 'compile-expression-to-js' that fails
@@ -2818,7 +2810,6 @@ e() {
28182810
''');
28192811

28202812
String library = 'package:hello/foo.dart';
2821-
String module = 'packages/hello/foo.dart';
28222813

28232814
File dillFile = new File('${tempDir.path}/foo.dart.dill');
28242815
File sourceFile = new File('${dillFile.path}.sources');
@@ -2854,11 +2845,7 @@ e() {
28542845
frontendServer.accept();
28552846

28562847
frontendServer.compileExpressionToJs(
2857-
expression: '',
2858-
libraryUri: library,
2859-
line: 2,
2860-
column: 1,
2861-
moduleName: module);
2848+
expression: '', libraryUri: library, line: 2, column: 1);
28622849
count += 1;
28632850
} else if (count == 1) {
28642851
// Second request is to 'compile-expression-to-js' that fails
@@ -2869,11 +2856,7 @@ e() {
28692856
});
28702857

28712858
frontendServer.compileExpressionToJs(
2872-
expression: '2+2',
2873-
libraryUri: library,
2874-
line: 2,
2875-
column: 1,
2876-
moduleName: module);
2859+
expression: '2+2', libraryUri: library, line: 2, column: 1);
28772860
count += 1;
28782861
} else if (count == 2) {
28792862
expect(result.errorsCount, equals(0));
@@ -2933,7 +2916,6 @@ e() {
29332916
''');
29342917

29352918
String library = 'package:hello/foo.dart';
2936-
String module = 'packages/hello/foo.dart';
29372919

29382920
File dillFile = new File('${tempDir.path}/foo.dart.dill');
29392921
File sourceFile = new File('${dillFile.path}.sources');
@@ -2979,8 +2961,7 @@ e() {
29792961
expression: 'const bool.fromEnvironment("dart.library.html")',
29802962
libraryUri: library,
29812963
line: 2,
2982-
column: 1,
2983-
moduleName: module);
2964+
column: 1);
29842965
count += 1;
29852966
} else {
29862967
expect(count, 1);
@@ -3030,7 +3011,6 @@ e() {
30303011
}
30313012
''');
30323013
String library = 'package:hello/foo.dart';
3033-
String module = 'packages/hello/foo.dart';
30343014

30353015
File dillFile = new File('${tempDir.path}/foo.dart.dill');
30363016
File sourceFile = new File('${dillFile.path}.sources');
@@ -3067,11 +3047,7 @@ e() {
30673047
frontendServer.accept();
30683048

30693049
frontendServer.compileExpressionToJs(
3070-
expression: '2+2',
3071-
libraryUri: library,
3072-
line: 2,
3073-
column: 1,
3074-
moduleName: module);
3050+
expression: '2+2', libraryUri: library, line: 2, column: 1);
30753051
count += 1;
30763052
} else if (count == 1) {
30773053
expect(result.errorsCount, equals(0));
@@ -3092,11 +3068,7 @@ e() {
30923068
expect(outputFile.lengthSync(), isPositive);
30933069

30943070
frontendServer.compileExpressionToJs(
3095-
expression: '2+2',
3096-
libraryUri: library,
3097-
line: 2,
3098-
column: 1,
3099-
moduleName: module);
3071+
expression: '2+2', libraryUri: library, line: 2, column: 1);
31003072
count += 1;
31013073
} else if (count == 3) {
31023074
expect(result.errorsCount, equals(0));
@@ -3739,10 +3711,13 @@ class FrontendServer {
37393711
required String libraryUri,
37403712
required int line,
37413713
required int column,
3742-
required String moduleName,
37433714
Uri? scriptUri,
37443715
Map<String, String>? jsFrameValues,
37453716
String boundaryKey = 'abc'}) {
3717+
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
3718+
// soft-deprecated, so even though it's unused, the request should still
3719+
// contain it so it can be parsed correctly.
3720+
final String moduleName = 'unused';
37463721
if (useJsonForCommunication) {
37473722
outputParser.expectSources = false;
37483723
inputStreamController.add('JSON_INPUT\n'.codeUnits);

‎pkg/frontend_server/test/src/javascript_bundle_test.dart‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,7 @@ void main() {
215215
expect(code, contains('sourceMappingURL=c.dart.lib.js.map'));
216216

217217
// Verify program compilers are created.
218-
final String moduleUrl = javaScriptBundler.urlForComponentUri(
219-
library.importUri, packageConfig);
220-
final String moduleName = javaScriptBundler.makeModuleName(moduleUrl);
221-
expect(compilers.keys, equals([moduleName]));
218+
expect(compilers.keys, equals([library.importUri.toString()]));
222219
});
223220

224221
test('converts package: uris into /packages/ uris', () async {

0 commit comments

Comments
 (0)