Skip to content

Commit 817306a

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
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]>
1 parent 527d982 commit 817306a

File tree

4 files changed

+57
-26
lines changed

4 files changed

+57
-26
lines changed

‎pkg/frontend_server/lib/frontend_server.dart‎

Lines changed: 17 additions & 14 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.
356+
/// at [line]:[column] to JavaScript in [moduleName].
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,6 +368,7 @@ abstract class CompilerInterface {
368368
/// expression.
369369
///
370370
/// Example values of parameters:
371+
/// [moduleName] is of the form '/packages/hello_world_main.dart'
371372
/// [jsFrameValues] is a map from js variable name to its primitive value
372373
/// or another variable name, for example
373374
/// { 'x': '1', 'y': 'y', 'o': 'null' }
@@ -382,6 +383,7 @@ abstract class CompilerInterface {
382383
int column,
383384
Map<String, String> jsModules,
384385
Map<String, String> jsFrameValues,
386+
String moduleName,
385387
String expression);
386388

387389
/// Communicates an error [msg] to the client.
@@ -1105,10 +1107,10 @@ class FrontendCompiler implements CompilerInterface {
11051107
}
11061108
}
11071109

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

11141116
@override
@@ -1119,6 +1121,7 @@ class FrontendCompiler implements CompilerInterface {
11191121
int column,
11201122
Map<String, String> jsModules,
11211123
Map<String, String> jsFrameValues,
1124+
String moduleName,
11221125
String expression) async {
11231126
_generator.accept();
11241127
errors.clear();
@@ -1127,17 +1130,18 @@ class FrontendCompiler implements CompilerInterface {
11271130
reportError('JavaScript bundler is null');
11281131
return;
11291132
}
1130-
if (!cachedProgramCompilers.containsKey(libraryUri)) {
1131-
reportError('Cannot find kernel2js compiler for $libraryUri.');
1133+
if (!cachedProgramCompilers.containsKey(moduleName)) {
1134+
reportError('Cannot find kernel2js compiler for $moduleName.');
11321135
return;
11331136
}
1137+
11341138
final String boundaryKey = generateV4UUID();
11351139
_outputStream.writeln('result $boundaryKey');
11361140

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

1140-
final Compiler kernel2jsCompiler = cachedProgramCompilers[libraryUri]!;
1144+
final Compiler kernel2jsCompiler = cachedProgramCompilers[moduleName]!;
11411145
IncrementalCompilerResult compilerResult = _generator.lastKnownGoodResult!;
11421146
Component component = compilerResult.component;
11431147
component.computeCanonicalNames();
@@ -1366,6 +1370,7 @@ class _CompileExpressionToJsRequest {
13661370
late int column;
13671371
Map<String, String> jsModules = <String, String>{};
13681372
Map<String, String> jsFrameValues = <String, String>{};
1373+
late String moduleName;
13691374
late String expression;
13701375
}
13711376

@@ -1589,9 +1594,7 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
15891594
}
15901595
break;
15911596
case _State.COMPILE_EXPRESSION_TO_JS_MODULENAME:
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.
1597+
compileExpressionToJsRequest.moduleName = string;
15951598
state = _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION;
15961599
break;
15971600
case _State.COMPILE_EXPRESSION_TO_JS_EXPRESSION:
@@ -1603,6 +1606,7 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
16031606
compileExpressionToJsRequest.column,
16041607
compileExpressionToJsRequest.jsModules,
16051608
compileExpressionToJsRequest.jsFrameValues,
1609+
compileExpressionToJsRequest.moduleName,
16061610
compileExpressionToJsRequest.expression);
16071611
state = _State.READY_FOR_INSTRUCTION;
16081612
break;
@@ -1740,14 +1744,12 @@ Future<void> processJsonInput(
17401744
int column = getValue<int>("column") ?? -1;
17411745
Map<String, String> jsModules = getMap("jsModules") ?? {};
17421746
Map<String, String> jsFrameValues = getMap("jsFrameValues") ?? {};
1747+
String moduleName = getValue<String>("moduleName") ?? "";
17431748

17441749
if (errorMessages.isNotEmpty) {
17451750
compiler.reportError("Errors: $errorMessages.");
17461751
return;
17471752
}
1748-
// TODO(https://github.com/dart-lang/sdk/issues/58265): `moduleName` is
1749-
// soft-deprecated, and therefore is unused.
1750-
unusedKeys.remove("moduleName");
17511753
if (unusedKeys.isNotEmpty) {
17521754
compiler.reportError("Errors: Send over unused data: $unusedKeys.");
17531755
}
@@ -1759,6 +1761,7 @@ Future<void> processJsonInput(
17591761
column,
17601762
jsModules,
17611763
jsFrameValues,
1764+
moduleName,
17621765
expression,
17631766
);
17641767
} 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[library.importUri.toString()] = compiler;
234+
kernel2JsCompilers[moduleName] = compiler;
235235

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

‎pkg/frontend_server/test/frontend_server_test.dart‎

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

799800
File dillFile = new File('${tempDir.path}/foo.dart.dill');
800801
File sourceFile = new File('${dillFile.path}.sources');
@@ -834,6 +835,7 @@ extension type Foo(int value) {
834835
libraryUri: library,
835836
line: 5,
836837
column: 3,
838+
moduleName: module,
837839
scriptUri: file.uri,
838840
jsFrameValues: {"f": "42"});
839841
count += 1;
@@ -848,6 +850,7 @@ extension type Foo(int value) {
848850
libraryUri: library,
849851
line: 11,
850852
column: 5,
853+
moduleName: module,
851854
scriptUri: file.uri,
852855
jsFrameValues: {r"$this": "42"});
853856
count += 1;
@@ -888,6 +891,7 @@ extension type Foo(int value) {
888891
];
889892

890893
String library = 'package:hello/foo.dart';
894+
String module = 'packages/hello/foo.dart';
891895

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

916920
frontendServer.compileExpressionToJs(
917-
expression: '', libraryUri: library, line: 1, column: 1);
921+
expression: '',
922+
libraryUri: library,
923+
line: 1,
924+
column: 1,
925+
moduleName: module);
918926
count += 1;
919927
} else if (count == 2) {
920928
// Third request is to 'compile-expression-to-js' that fails
@@ -2810,6 +2818,7 @@ e() {
28102818
''');
28112819

28122820
String library = 'package:hello/foo.dart';
2821+
String module = 'packages/hello/foo.dart';
28132822

28142823
File dillFile = new File('${tempDir.path}/foo.dart.dill');
28152824
File sourceFile = new File('${dillFile.path}.sources');
@@ -2845,7 +2854,11 @@ e() {
28452854
frontendServer.accept();
28462855

28472856
frontendServer.compileExpressionToJs(
2848-
expression: '', libraryUri: library, line: 2, column: 1);
2857+
expression: '',
2858+
libraryUri: library,
2859+
line: 2,
2860+
column: 1,
2861+
moduleName: module);
28492862
count += 1;
28502863
} else if (count == 1) {
28512864
// Second request is to 'compile-expression-to-js' that fails
@@ -2856,7 +2869,11 @@ e() {
28562869
});
28572870

28582871
frontendServer.compileExpressionToJs(
2859-
expression: '2+2', libraryUri: library, line: 2, column: 1);
2872+
expression: '2+2',
2873+
libraryUri: library,
2874+
line: 2,
2875+
column: 1,
2876+
moduleName: module);
28602877
count += 1;
28612878
} else if (count == 2) {
28622879
expect(result.errorsCount, equals(0));
@@ -2916,6 +2933,7 @@ e() {
29162933
''');
29172934

29182935
String library = 'package:hello/foo.dart';
2936+
String module = 'packages/hello/foo.dart';
29192937

29202938
File dillFile = new File('${tempDir.path}/foo.dart.dill');
29212939
File sourceFile = new File('${dillFile.path}.sources');
@@ -2961,7 +2979,8 @@ e() {
29612979
expression: 'const bool.fromEnvironment("dart.library.html")',
29622980
libraryUri: library,
29632981
line: 2,
2964-
column: 1);
2982+
column: 1,
2983+
moduleName: module);
29652984
count += 1;
29662985
} else {
29672986
expect(count, 1);
@@ -3011,6 +3030,7 @@ e() {
30113030
}
30123031
''');
30133032
String library = 'package:hello/foo.dart';
3033+
String module = 'packages/hello/foo.dart';
30143034

30153035
File dillFile = new File('${tempDir.path}/foo.dart.dill');
30163036
File sourceFile = new File('${dillFile.path}.sources');
@@ -3047,7 +3067,11 @@ e() {
30473067
frontendServer.accept();
30483068

30493069
frontendServer.compileExpressionToJs(
3050-
expression: '2+2', libraryUri: library, line: 2, column: 1);
3070+
expression: '2+2',
3071+
libraryUri: library,
3072+
line: 2,
3073+
column: 1,
3074+
moduleName: module);
30513075
count += 1;
30523076
} else if (count == 1) {
30533077
expect(result.errorsCount, equals(0));
@@ -3068,7 +3092,11 @@ e() {
30683092
expect(outputFile.lengthSync(), isPositive);
30693093

30703094
frontendServer.compileExpressionToJs(
3071-
expression: '2+2', libraryUri: library, line: 2, column: 1);
3095+
expression: '2+2',
3096+
libraryUri: library,
3097+
line: 2,
3098+
column: 1,
3099+
moduleName: module);
30723100
count += 1;
30733101
} else if (count == 3) {
30743102
expect(result.errorsCount, equals(0));
@@ -3711,13 +3739,10 @@ class FrontendServer {
37113739
required String libraryUri,
37123740
required int line,
37133741
required int column,
3742+
required String moduleName,
37143743
Uri? scriptUri,
37153744
Map<String, String>? jsFrameValues,
37163745
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';
37213746
if (useJsonForCommunication) {
37223747
outputParser.expectSources = false;
37233748
inputStreamController.add('JSON_INPUT\n'.codeUnits);

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

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

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

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

0 commit comments

Comments
 (0)