Skip to content

Commit c338941

Browse files
lauraharkercopybara-github
authored andcommitted
Support @closureUnaware transpilation using runtime libraries as 'external' names
This adds a new mode to the RuntimeJsLibManager class specifically for @closureUnaware transpilation. Normally transpilation passes reference runtime library fields by the original, qualified field name, as it appears in the JS file. Examples - "$jscomp.global", "$jscomp.inherits". In optimized builds: the actual runtime library code is compiled together with the transpiled code. In --skip_non_transpilation_passes/transpile only builds: the actual runtime library code must be loaded separately, usually via a regular library dependency. However, for transpilation of @closureUnaware blocks within a larger optimized build, we need to both 1) share runtime library code with the regular optimized build & also minify it, for code size reasons but 2) not break references to the runtime library code from within @closureUnaware blocks. Naively referring to runtime libraries in @closureUnaware code will break after variable & property renaming runs on the main AST. So instead, we have to treat the runtime libraries as unknown external code for @closureUnaware transpilation - which is what externs let us do. Then in a followup CL, we'll actually inject the runtime field definitions into the @closureUnaware function as arguments. PiperOrigin-RevId: 851364465
1 parent 34e67e3 commit c338941

File tree

7 files changed

+259
-40
lines changed

7 files changed

+259
-40
lines changed

src/com/google/javascript/jscomp/AstFactory.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -781,14 +781,17 @@ Node createQName(StaticScope scope, JsLibField field) {
781781
String qname = field.assertInjected().qualifiedName();
782782
List<String> parts = DOT_SPLITTER.splitToList(qname);
783783
String baseName = checkNotNull(Iterables.getFirst(parts, null));
784-
checkState(baseName.equals("$jscomp"), "Unexpected Field name %s", baseName);
785-
Node baseNameNode = createJscomp();
786-
setJSTypeOrColor(type(unknownType, StandardColors.UNKNOWN), baseNameNode);
784+
checkState(baseName.startsWith("$jscomp"), "Unexpected Field name %s", baseName);
785+
786+
Node baseNameNode =
787+
baseName.equals("$jscomp")
788+
? createJscomp()
789+
: createName(baseName, type(unknownType, StandardColors.UNKNOWN));
787790
if (lifeCycleStage.isNormalized()) {
788791
baseNameNode.putBooleanProp(Node.IS_CONSTANT_NAME, true);
789792
}
790793
Iterable<String> propertyNames = Iterables.skip(parts, 1);
791-
return createQName(scope, "$jscomp", baseNameNode, propertyNames);
794+
return createQName(scope, baseName, baseNameNode, propertyNames);
792795
}
793796

794797
Node createQNameWithUnknownType(String qname) {

src/com/google/javascript/jscomp/ClosureUnawareOptions.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,14 @@ private void setTranspilationOptions() {
4545
// transpilation, in the
4646
// case we can't prove it's not a subclass of an ES6 class (not being transpiled)
4747
shadowOptions.setRewritePolyfills(false); // for now, we ignore polyfills that may be needed.
48-
shadowOptions.setRuntimeLibraryMode(RuntimeJsLibManager.RuntimeLibraryMode.NO_OP);
48+
switch (original.getRuntimeLibraryMode()) {
49+
case NO_OP ->
50+
shadowOptions.setRuntimeLibraryMode(RuntimeJsLibManager.RuntimeLibraryMode.NO_OP);
51+
case RECORD_ONLY, RECORD_AND_VALIDATE_FIELDS, INJECT ->
52+
shadowOptions.setRuntimeLibraryMode(
53+
RuntimeJsLibManager.RuntimeLibraryMode.EXTERN_FIELD_NAMES);
54+
case EXTERN_FIELD_NAMES -> throw new AssertionError();
55+
}
4956
shadowOptions.setOutputFeatureSet(original.getOutputFeatureSet());
5057
}
5158

src/com/google/javascript/jscomp/Compiler.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,10 @@ public void initChunks(List<SourceFile> externs, List<JSChunk> chunks, CompilerO
842842
options.getRuntimeLibraryMode(),
843843
this::loadResourceContents,
844844
changeTracker,
845-
this::getNodeForRuntimeCodeInsertion);
845+
options.getRuntimeLibraryMode()
846+
== RuntimeJsLibManager.RuntimeLibraryMode.EXTERN_FIELD_NAMES
847+
? this::getSynthesizedExternsRoot
848+
: this::getNodeForRuntimeCodeInsertion);
846849
}
847850

848851
@Override
@@ -3938,6 +3941,11 @@ CompilerInput getSynthesizedExternsInput() {
39383941
return input;
39393942
}
39403943

3944+
private Node getSynthesizedExternsRoot() {
3945+
CompilerInput externsInput = getSynthesizedExternsInput();
3946+
return externsInput.getAstRoot(this);
3947+
}
3948+
39413949
@Override
39423950
CompilerInput getSynthesizedTypeSummaryInput() {
39433951
if (syntheticTypeSummaryInput != null) {

src/com/google/javascript/jscomp/js/RuntimeJsLibManager.java

Lines changed: 86 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkArgument;
2020
import static com.google.common.base.Preconditions.checkNotNull;
2121
import static com.google.common.base.Preconditions.checkState;
22+
import static com.google.common.collect.ImmutableList.toImmutableList;
2223

2324
import com.google.common.base.Splitter;
2425
import com.google.common.base.Strings;
@@ -27,6 +28,7 @@
2728
import com.google.errorprone.annotations.CanIgnoreReturnValue;
2829
import com.google.javascript.jscomp.ChangeTracker;
2930
import com.google.javascript.jscomp.resources.ResourceLoader;
31+
import com.google.javascript.rhino.IR;
3032
import com.google.javascript.rhino.Node;
3133
import java.util.LinkedHashMap;
3234
import java.util.LinkedHashSet;
@@ -61,6 +63,13 @@ public enum RuntimeLibraryMode {
6163
// inject a runtime library, and will also validate injection in JsLibField::assertInjected, but
6264
// does not modify the AST.
6365
RECORD_AND_VALIDATE_FIELDS,
66+
// for @closureUnaware nested compilation. When asked to inject a field, the compiler adds an
67+
// @extern definition of the field name to prevent misoptimization, and uses a different
68+
// identifier from the original uncompiled field name.
69+
// The RuntimeJsLibManager is not responsible for actually defining the external field name
70+
// definitions; the code using EXTERN_FIELD_NAMES mode must enure that everything field from
71+
// getExternedFields() is actually loaded at runtime.
72+
EXTERN_FIELD_NAMES,
6473
}
6574

6675
/** Loads /js resources into AST format */
@@ -155,6 +164,18 @@ public ImmutableList<String> getInjectedLibraries() {
155164
return ImmutableList.copyOf(injectedLibs);
156165
}
157166

167+
/**
168+
* Returns a list of all fields previously injected, via one of the other methods on this class.
169+
*/
170+
public ImmutableList<ExternedField> getExternedFields() {
171+
if (this.mode != RuntimeLibraryMode.EXTERN_FIELD_NAMES) {
172+
return ImmutableList.of();
173+
}
174+
return internedFields.values().stream()
175+
.filter(InternalField::isInjected)
176+
.collect(toImmutableList());
177+
}
178+
158179
private InternalField createField(String fieldName) {
159180
checkArgument(!fieldName.isEmpty(), fieldName);
160181

@@ -166,27 +187,43 @@ private InternalField createField(String fieldName) {
166187

167188
String resourceName =
168189
checkNotNull(FieldsTable.INSTANCE.get(propName), "Cannot find definition of %s", fieldName);
169-
// TODO: b/421971366 - allow @closureUnaware transpilation to use a different local name for
170-
// fields.
171-
return new InternalField(resourceName, fieldName);
190+
191+
String compiledName =
192+
switch (mode) {
193+
case EXTERN_FIELD_NAMES -> fieldName.replace('.', '_');
194+
case NO_OP, RECORD_ONLY, RECORD_AND_VALIDATE_FIELDS, INJECT -> fieldName;
195+
};
196+
197+
return new InternalField(resourceName, compiledName, fieldName);
172198
}
173199

174200
/**
175201
* A $jscomp.* field (could be a method, class, or any arbitrary value) that exists in some
176202
* runtime library under the js/ directory.
177203
*/
178-
private final class InternalField implements InjectedJsLibField {
204+
private final class InternalField implements ExternedField {
179205
/** The file containing this field, e.g. "es6/generator". */
180206
private final String resourceName;
181207

182-
/** The name by which compiler passes should refer to this field. */
208+
/**
209+
* The name by which compiler passes should refer to this field, e.g. `$jscomp.inherits` or
210+
* `$jscomp_inherits`.
211+
*/
183212
private final String qualifiedName;
184213

214+
/** The original fully qualified name of this field, e.g. `$jscomp.inherits`. */
215+
private final String uncompiledName;
216+
185217
private boolean injected = false;
186218

187-
private InternalField(String resourceName, String qualifiedName) {
219+
private InternalField(String resourceName, String qualifiedName, String uncompiledName) {
188220
this.resourceName = resourceName;
189221
this.qualifiedName = qualifiedName;
222+
this.uncompiledName = uncompiledName;
223+
}
224+
225+
private boolean isInjected() {
226+
return this.injected;
190227
}
191228

192229
private void markInjected() {
@@ -198,8 +235,8 @@ private void markInjected() {
198235
public InternalField assertInjected() {
199236
switch (mode) {
200237
case NO_OP, RECORD_ONLY -> {}
201-
case RECORD_AND_VALIDATE_FIELDS, INJECT ->
202-
checkState(this.injected, "Field %s is not injected", this.qualifiedName);
238+
case RECORD_AND_VALIDATE_FIELDS, INJECT, EXTERN_FIELD_NAMES ->
239+
checkState(this.injected, "Field %s is not injected", this.uncompiledName);
203240
}
204241
return this;
205242
}
@@ -214,13 +251,23 @@ public boolean matches(Node node) {
214251

215252
@Override
216253
public String toString() {
217-
return Strings.lenientFormat("Field<%s, %s>", qualifiedName, resourceName);
254+
return Strings.lenientFormat("Field<%s, %s>", uncompiledName, resourceName);
218255
}
219256

220257
@Override
221258
public String qualifiedName() {
222259
return this.qualifiedName;
223260
}
261+
262+
@Override
263+
public String uncompiledName() {
264+
return this.uncompiledName;
265+
}
266+
267+
@Override
268+
public String resourceName() {
269+
return this.resourceName;
270+
}
224271
}
225272

226273
/**
@@ -245,6 +292,8 @@ private InternalField getJsLibFieldInternal(String fieldName) {
245292
public interface JsLibField {
246293
boolean matches(Node node);
247294

295+
String resourceName();
296+
248297
InjectedJsLibField assertInjected();
249298
}
250299

@@ -256,13 +305,40 @@ public interface JsLibField {
256305
* to prevent code from creating new AST references to a field that's not actually injected (yet).
257306
*/
258307
public interface InjectedJsLibField extends JsLibField {
308+
/** Returns the name of the field as it is available to the compiler. */
259309
String qualifiedName();
260310
}
261311

312+
/**
313+
* A {@link InjectedJsLibField} that was injected in {@link RuntimeLibraryMode#EXTERN_FIELD_NAMES}
314+
* mode, and so has both a regular "qualified name" by which transpilation passes should refer to
315+
* it - this is the externed name - but also provides the uncompiled, raw name of the field.
316+
*
317+
* <p>Only {@link ExternedField} instances provide access to the uncompiled name. Most use sites
318+
* should use the compiled/qualified name, which may or may not equal the uncompiled name. So to
319+
* try and minimize confusion, we only expose the uncompiled name when this is actually an
320+
* external field injection.
321+
*/
322+
public interface ExternedField extends InjectedJsLibField {
323+
324+
/** Returns the uncompiled name of the field, as it is seen in the raw runtime lib. */
325+
String uncompiledName();
326+
}
327+
262328
/** Injects the runtime library that defines the given $jscomp.* field. */
263329
public void injectLibForField(String fieldName) {
264330
InternalField field = getJsLibFieldInternal(fieldName);
331+
if (field.isInjected()) {
332+
return; // already done.
333+
}
265334
field.markInjected();
335+
if (this.mode == RuntimeLibraryMode.EXTERN_FIELD_NAMES) {
336+
Node externRoot = nodeForCodeInsertion.get();
337+
checkState(externRoot.isFromExterns(), externRoot);
338+
Node declaration = IR.var(IR.name(field.qualifiedName()));
339+
declaration.getFirstChild().putBooleanProp(Node.IS_CONSTANT_NAME, true);
340+
externRoot.addChildToBack(declaration);
341+
}
266342
ensureLibraryInjected(field.resourceName, /* force= */ false);
267343
}
268344

@@ -287,7 +363,7 @@ public void injectLibForField(String fieldName) {
287363
case NO_OP -> {
288364
return lastInjectedLibrary;
289365
}
290-
case RECORD_ONLY, RECORD_AND_VALIDATE_FIELDS -> {
366+
case RECORD_ONLY, RECORD_AND_VALIDATE_FIELDS, EXTERN_FIELD_NAMES -> {
291367
this.recordLibraryInjected(resourceName);
292368
return lastInjectedLibrary;
293369
}

test/com/google/javascript/jscomp/AstFactoryTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.javascript.rhino.IR;
3838
import com.google.javascript.rhino.Node;
3939
import com.google.javascript.rhino.StaticScope;
40+
import com.google.javascript.rhino.StaticSourceFile.SourceKind;
4041
import com.google.javascript.rhino.Token;
4142
import com.google.javascript.rhino.jstype.FunctionType;
4243
import com.google.javascript.rhino.jstype.JSType;
@@ -2076,6 +2077,29 @@ public void testCreateRuntimeField_succeeds() {
20762077
assertNode(result).matchesQualifiedName("$jscomp.global");
20772078
}
20782079

2080+
@Test
2081+
public void testCreateRuntimeField_inExternMode_succeeds_andUsesUnderscoredName() {
2082+
// Given
2083+
Node externs = IR.script();
2084+
externs.setStaticSourceFile(SourceFile.fromCode("externs.js", "", SourceKind.EXTERN));
2085+
this.runtimeJsLibManager =
2086+
RuntimeJsLibManager.create(
2087+
RuntimeJsLibManager.RuntimeLibraryMode.EXTERN_FIELD_NAMES,
2088+
new TestResourceProvider(),
2089+
compiler.getChangeTracker(),
2090+
() -> externs);
2091+
AstFactory astFactory = createTestAstFactoryWithoutTypes();
2092+
StaticScope scope = new MapBasedScope(ImmutableMap.of());
2093+
RuntimeJsLibManager.JsLibField field = runtimeJsLibManager.getJsLibField("$jscomp.global");
2094+
runtimeJsLibManager.injectLibForField("$jscomp.global");
2095+
2096+
// When
2097+
var result = astFactory.createQName(scope, field);
2098+
2099+
// Then
2100+
assertNode(result).matchesName("$jscomp_global");
2101+
}
2102+
20792103
@Test
20802104
public void testCreateJscompMakeIteratorCall_throwsIfJscompMakeIteratorNotInjected() {
20812105
// Given

test/com/google/javascript/jscomp/TranspileAndOptimizeClosureUnawareTest.java

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717
package com.google.javascript.jscomp;
1818

1919
import static com.google.common.collect.ImmutableList.toImmutableList;
20-
import static com.google.common.truth.Truth.assertThat;
2120
import static com.google.javascript.jscomp.CompilerTestCase.srcs;
2221
import static java.util.stream.Collectors.joining;
23-
import static org.junit.Assert.assertThrows;
2422

2523
import com.google.common.collect.ImmutableList;
2624
import com.google.javascript.rhino.Node;
@@ -293,28 +291,43 @@ public void transpileOnlyMode_doesNotRunConstantFolding() {
293291
@Test
294292
public void classInheritance_injectsRequiredRuntimeLibraries() {
295293
setLanguageOut(CompilerOptions.LanguageMode.ECMASCRIPT5);
296-
// TODO: b/830636484 - enable runtime library injection and fix this test.
297-
var ex =
298-
assertThrows(
299-
Exception.class,
300-
() ->
301-
test(
302-
srcs(
303-
"""
304-
/** @fileoverview @closureUnaware */
305-
goog.module('test');
306-
/** @closureUnaware */
307-
(function() {
308-
class Parent {
309-
method() {
310-
return 10;
311-
}
312-
}
313-
class Child extends Parent {}
314-
return new Child();
315-
}).call(globalThis);
316-
""")));
317-
assertThat(ex).hasCauseThat().hasMessageThat().contains("NAME $jscomp");
294+
295+
test(
296+
srcs(
297+
"""
298+
/** @fileoverview @closureUnaware */
299+
goog.module('test');
300+
/** @closureUnaware */
301+
(function() {
302+
class Parent {
303+
method() {
304+
return 10;
305+
}
306+
}
307+
class Child extends Parent {}
308+
return new Child();
309+
}).call(globalThis);
310+
"""),
311+
expected(
312+
// TODO: b/830636484 inject $jscomp_inherits as a parameter in the closure unaware fn.
313+
"""
314+
/** @fileoverview @closureUnaware */
315+
goog.module("test");
316+
/** @closureUnaware */
317+
(function() {
318+
/** @constructor */
319+
var a = function() {};
320+
a.prototype.method = function() {
321+
return 10;
322+
};
323+
/** @constructor */
324+
var b = function() {
325+
return a.apply(this, arguments) || this;
326+
};
327+
$jscomp_inherits(b, a);
328+
return new b();
329+
}).call(globalThis);
330+
"""));
318331
}
319332

320333
@Test

0 commit comments

Comments
 (0)