Skip to content

Commit 32565d1

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
Reland "[frontend_server] Soft-deprecate moduleName for compileExpressionToJS"
This reverts commit 817306a. Reason for revert: Later patchset includes fix for `frontend_server_circular_evaluate_test`. The fix is to cache the bundle/program compiler for all the libraries in the strongly connected component, and not just for the initial one that determines the component uri. Original change's description: > Revert "[frontend_server] Soft-deprecate moduleName for compileExpressionToJS" > > This reverts commit 294a50f. > > Reason for revert: This breaks `frontend_server_circular_evaluate_test` in DWDS. > > Original change's description: > > [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]> > > Change-Id: I74b096f7ebc322c9d4428d236ab226ef1adcb6b9 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396567 > Reviewed-by: Nicholas Shahan <[email protected]> > Reviewed-by: Johnni Winther <[email protected]> > Bot-Commit: Rubber Stamper <[email protected]> > Commit-Queue: Srujan Gaddam <[email protected]> Change-Id: I45852c29a0b5735976f6a5f649f3ee84b26ef76c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396572 Reviewed-by: Nicholas Shahan <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Srujan Gaddam <[email protected]>
1 parent 6a1d3de commit 32565d1

File tree

4 files changed

+30
-60
lines changed

4 files changed

+30
-60
lines changed

‎pkg/frontend_server/lib/frontend_server.dart‎

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

355355
/// Compiles [expression] in library [libraryUri] and file [scriptUri]
356-
/// at [line]:[column] to JavaScript in [moduleName].
356+
/// at [line]:[column] to JavaScript.
357357
///
358358
/// [libraryUri] and [scriptUri] can be the same, but if for instance
359359
/// evaluating expressions in a part file the [libraryUri] will be the uri of
@@ -368,7 +368,6 @@ abstract class CompilerInterface {
368368
/// expression.
369369
///
370370
/// Example values of parameters:
371-
/// [moduleName] is of the form '/packages/hello_world_main.dart'
372371
/// [jsFrameValues] is a map from js variable name to its primitive value
373372
/// or another variable name, for example
374373
/// { 'x': '1', 'y': 'y', 'o': 'null' }
@@ -383,7 +382,6 @@ abstract class CompilerInterface {
383382
int column,
384383
Map<String, String> jsModules,
385384
Map<String, String> jsFrameValues,
386-
String moduleName,
387385
String expression);
388386

389387
/// Communicates an error [msg] to the client.
@@ -1107,10 +1105,10 @@ class FrontendCompiler implements CompilerInterface {
11071105
}
11081106
}
11091107

1110-
/// Program compilers per module.
1108+
/// Mapping of libraries to their program compiler.
11111109
///
1112-
/// Produced during initial compilation of the module to JavaScript,
1113-
/// cached to be used for expression compilation in [compileExpressionToJs].
1110+
/// Produced during initial compilation of the component to JavaScript, cached
1111+
/// to be used for expression compilation in [compileExpressionToJs].
11141112
final Map<String, Compiler> cachedProgramCompilers = {};
11151113

11161114
@override
@@ -1121,7 +1119,6 @@ class FrontendCompiler implements CompilerInterface {
11211119
int column,
11221120
Map<String, String> jsModules,
11231121
Map<String, String> jsFrameValues,
1124-
String moduleName,
11251122
String expression) async {
11261123
_generator.accept();
11271124
errors.clear();
@@ -1130,18 +1127,17 @@ class FrontendCompiler implements CompilerInterface {
11301127
reportError('JavaScript bundler is null');
11311128
return;
11321129
}
1133-
if (!cachedProgramCompilers.containsKey(moduleName)) {
1134-
reportError('Cannot find kernel2js compiler for $moduleName.');
1130+
if (!cachedProgramCompilers.containsKey(libraryUri)) {
1131+
reportError('Cannot find kernel2js compiler for $libraryUri.');
11351132
return;
11361133
}
1137-
11381134
final String boundaryKey = generateV4UUID();
11391135
_outputStream.writeln('result $boundaryKey');
11401136

11411137
_processedOptions.ticker
1142-
.logMs('Compiling expression to JavaScript in $moduleName');
1138+
.logMs('Compiling expression to JavaScript in $libraryUri');
11431139

1144-
final Compiler kernel2jsCompiler = cachedProgramCompilers[moduleName]!;
1140+
final Compiler kernel2jsCompiler = cachedProgramCompilers[libraryUri]!;
11451141
IncrementalCompilerResult compilerResult = _generator.lastKnownGoodResult!;
11461142
Component component = compilerResult.component;
11471143
component.computeCanonicalNames();
@@ -1370,7 +1366,6 @@ class _CompileExpressionToJsRequest {
13701366
late int column;
13711367
Map<String, String> jsModules = <String, String>{};
13721368
Map<String, String> jsFrameValues = <String, String>{};
1373-
late String moduleName;
13741369
late String expression;
13751370
}
13761371

@@ -1594,7 +1589,9 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
15941589
}
15951590
break;
15961591
case _State.COMPILE_EXPRESSION_TO_JS_MODULENAME:
1597-
compileExpressionToJsRequest.moduleName = string;
1592+
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
1593+
// soft-deprecated, so we still need to consume the string value, but it
1594+
// is unused.
15981595
state = _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION;
15991596
break;
16001597
case _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION:
@@ -1606,7 +1603,6 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
16061603
compileExpressionToJsRequest.column,
16071604
compileExpressionToJsRequest.jsModules,
16081605
compileExpressionToJsRequest.jsFrameValues,
1609-
compileExpressionToJsRequest.moduleName,
16101606
compileExpressionToJsRequest.expression);
16111607
state = _State.READY_FOR_INSTRUCTION;
16121608
break;
@@ -1744,12 +1740,14 @@ Future<void> processJsonInput(
17441740
int column = getValue<int>("column") ?? -1;
17451741
Map<String, String> jsModules = getMap("jsModules") ?? {};
17461742
Map<String, String> jsFrameValues = getMap("jsFrameValues") ?? {};
1747-
String moduleName = getValue<String>("moduleName") ?? "";
17481743

17491744
if (errorMessages.isNotEmpty) {
17501745
compiler.reportError("Errors: $errorMessages.");
17511746
return;
17521747
}
1748+
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
1749+
// soft-deprecated, and therefore is unused.
1750+
unusedKeys.remove("moduleName");
17531751
if (unusedKeys.isNotEmpty) {
17541752
compiler.reportError("Errors: Send over unused data: $unusedKeys.");
17551753
}
@@ -1761,7 +1759,6 @@ Future<void> processJsonInput(
17611759
column,
17621760
jsModules,
17631761
jsFrameValues,
1764-
moduleName,
17651762
expression,
17661763
);
17671764
} else {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ class IncrementalJavaScriptBundler {
179179
int metadataOffset = 0;
180180
int symbolsOffset = 0;
181181
final Map<String, Map<String, List<int>>> manifest = {};
182-
final Set<Uri> visited = {};
182+
final Map<Uri, Compiler> visited = {};
183183
final Map<String, Compiler> kernel2JsCompilers = {};
184184

185185
for (Library library in _currentComponent.libraries) {
@@ -189,10 +189,10 @@ class IncrementalJavaScriptBundler {
189189
}
190190
final Uri moduleUri =
191191
_strongComponents.moduleAssignment[library.importUri]!;
192-
if (visited.contains(moduleUri)) {
192+
if (visited.containsKey(moduleUri)) {
193+
kernel2JsCompilers[library.importUri.toString()] = visited[moduleUri]!;
193194
continue;
194195
}
195-
visited.add(moduleUri);
196196

197197
final Component summaryComponent = _uriToComponent[moduleUri]!;
198198

@@ -231,7 +231,8 @@ 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;
235+
visited[moduleUri] = compiler;
235236

236237
String? sourceMapBase;
237238
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)