Skip to content

Commit b7294bd

Browse files
lauraharkercopybara-github
authored andcommitted
Add an option to transpile ES6 constructors using Reflect.construct polyfill when the superclass is not statically analyzable.
This controls whether `super()` calls in transpiled ES6 classes should use `$jscomp.construct` (a wrapper around `Reflect.construct`) when the superclass is not statically known to be an ES5-style function. The default transpilation will fail if subclassing a (non-transpiled) ES6 class, such as a class loaded at runtime from a different pre-minified bundle. Ideally we would always set this option. For now, I just plan to enable it for @closureUnaware transpilation. PiperOrigin-RevId: 852503810
1 parent c54741b commit b7294bd

File tree

5 files changed

+191
-8
lines changed

5 files changed

+191
-8
lines changed

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,13 @@ public static enum OutputJs {
878878
*/
879879
private LanguageMode injectPolyfillsNewerThan = null;
880880

881+
/**
882+
* Whether to emit the safe but slower/more verbose transpilation of class super, when the
883+
* compiler cannot statically determine whether the superclass is an ES6 class.
884+
*/
885+
private Es6SubclassTranspilation es6SubclassTranspilation =
886+
Es6SubclassTranspilation.CONCISE_UNSAFE;
887+
881888
/** Whether to instrument reentrant functions for AsyncContext. */
882889
private boolean instrumentAsyncContext = false;
883890

@@ -2789,6 +2796,35 @@ public boolean getInstrumentAsyncContext() {
27892796
return this.instrumentAsyncContext;
27902797
}
27912798

2799+
/**
2800+
* Configures the transpiled output for ES6 classes when targeting ES5.
2801+
*
2802+
* <p>This is needed for transpilation of ES6 classes that extend some unknown superclass, in case
2803+
* that superclass is still a native ES6 class at runtime (not transpiled to ES5).
2804+
*
2805+
* <p>The default is {@link Es6SubclassTranspilation#CONCISE_UNSAFE}, which assumes that if a
2806+
* superclass in an extends clause cannot be statically analyzed, it's safe to assume it is an ES5
2807+
* class.
2808+
*
2809+
* <p>The "safe" behavior relies on {@code $jscomp.construct}, which is our wrapper for {@code
2810+
* Reflect.construct}. This is correct, but has the disadvantage of pulling in more helper
2811+
* utilities into the compiled output and of the transpiled output being slightly larger.
2812+
*
2813+
* <p>TODO: b/36789413 - always enable the "safe" transpilation.
2814+
*/
2815+
public enum Es6SubclassTranspilation {
2816+
CONCISE_UNSAFE,
2817+
SAFE_REFLECT_CONSTRUCT
2818+
}
2819+
2820+
public void setEs6SubclassTranspilation(Es6SubclassTranspilation es6SubclassTranspilation) {
2821+
this.es6SubclassTranspilation = es6SubclassTranspilation;
2822+
}
2823+
2824+
public Es6SubclassTranspilation getEs6SubclassTranspilation() {
2825+
return this.es6SubclassTranspilation;
2826+
}
2827+
27922828
/** Sets list of libraries to always inject, even if not needed. */
27932829
public void setForceLibraryInjection(Iterable<String> libraries) {
27942830
this.forceLibraryInjection = ImmutableList.copyOf(libraries);
@@ -2985,6 +3021,7 @@ public String toString() {
29853021
.add("errorFormat", errorFormat)
29863022
.add("errorHandler", errorHandler)
29873023
.add("es6ModuleTranspilation", es6ModuleTranspilation)
3024+
.add("es6SubclassTranspilation", es6SubclassTranspilation)
29883025
.add("exportLocalPropertyDefinitions", exportLocalPropertyDefinitions)
29893026
.add("exportTestFunctions", exportTestFunctions)
29903027
.add("externExportsPath", externExportsPath)

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static com.google.javascript.jscomp.AstFactory.type;
2222

2323
import com.google.common.collect.Iterables;
24+
import com.google.javascript.jscomp.CompilerOptions.Es6SubclassTranspilation;
2425
import com.google.javascript.jscomp.GlobalNamespace.Name;
2526
import com.google.javascript.jscomp.GlobalNamespace.Ref;
2627
import com.google.javascript.jscomp.colors.Color;
@@ -62,8 +63,10 @@ private static final class ConstructorData {
6263
private GlobalNamespace globalNamespace;
6364
private final StaticScope transpilationNamespace;
6465
private final UniqueIdSupplier uniqueIdSupplier;
66+
private final Es6SubclassTranspilation es6SubclassTranspilation;
6567

66-
public Es6ConvertSuperConstructorCalls(AbstractCompiler compiler) {
68+
public Es6ConvertSuperConstructorCalls(
69+
AbstractCompiler compiler, Es6SubclassTranspilation es6SubclassTranspilation) {
6770
this.compiler = compiler;
6871
this.astFactory = compiler.createAstFactory();
6972
this.transpilationNamespace = compiler.getTranspilationNamespace();
@@ -73,6 +76,7 @@ public Es6ConvertSuperConstructorCalls(AbstractCompiler compiler) {
7376
var runtimeJsLibManager = compiler.getRuntimeJsLibManager();
7477
this.jscompInherits = runtimeJsLibManager.getJsLibField("$jscomp.inherits");
7578
this.jscompConstruct = runtimeJsLibManager.getJsLibField("$jscomp.construct");
79+
this.es6SubclassTranspilation = es6SubclassTranspilation;
7680
}
7781

7882
@Override
@@ -305,12 +309,17 @@ private void rewriteSuperAndPreserveReturnedThis(
305309
// 1. We must use the value it returns, if defined, as the 'this' value in the constructor
306310
// we're currently transpiling, and we must also return it from this constructor.
307311
// 2. It may be an ES6 class defined outside of the sources we can see.
308-
//
312+
if (es6SubclassTranspilation == Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT) {
313+
// To safely extend the ES5 classes we're generating here, we must use
314+
// `$jscomp.construct`, which is our wrapper around `Reflect.construct`.
315+
rewriteSuperCallsToJsCompConstructCalls(
316+
constructor, superCalls, superClassNameNode, thisType, uniqueSuperThisName);
317+
return;
318+
}
309319
// The code below works as long as the class we're extending is an ES5 class, but will
310320
// break if we're extending an ES6 class (#2), because it calls the superclass constructor
311321
// without using `new` or `Reflect.construct()`
312-
// TODO(b/36789413): We should use $jscomp.construct() here to avoid breakage when extending
313-
// an ES6 class.
322+
// TODO(b/36789413): we should always use `$jscomp.construct`.
314323
Node constructorBody = checkNotNull(constructor.getChildAtIndex(2));
315324
Node firstStatement = constructorBody.getFirstChild();
316325
Node firstSuperCall = superCalls.get(0);

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import com.google.auto.value.AutoBuilder;
2525
import com.google.common.base.Preconditions;
26+
import com.google.javascript.jscomp.CompilerOptions.Es6SubclassTranspilation;
2627
import com.google.javascript.jscomp.colors.Color;
2728
import com.google.javascript.jscomp.colors.StandardColors;
2829
import com.google.javascript.jscomp.js.RuntimeJsLibManager.JsLibField;
@@ -50,11 +51,13 @@ public final class Es6RewriteClass implements NodeTraversal.Callback, CompilerPa
5051
private final StaticScope transpilationNamespace;
5152
private final JsLibField jscompInherits;
5253

53-
public Es6RewriteClass(AbstractCompiler compiler) {
54+
public Es6RewriteClass(
55+
AbstractCompiler compiler, Es6SubclassTranspilation es6SubclassTranspilation) {
5456
this.compiler = compiler;
5557
this.astFactory = compiler.createAstFactory();
5658
this.transpilationNamespace = compiler.getTranspilationNamespace();
57-
this.convertSuperConstructorCalls = new Es6ConvertSuperConstructorCalls(compiler);
59+
this.convertSuperConstructorCalls =
60+
new Es6ConvertSuperConstructorCalls(compiler, es6SubclassTranspilation);
5861
this.jscompInherits = compiler.getRuntimeJsLibManager().getJsLibField("$jscomp.inherits");
5962
}
6063

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,10 @@ public static void addRewritePolyfillPass(PassListBuilder passes) {
265265
static final PassFactory es6RewriteClass =
266266
PassFactory.builder()
267267
.setName("Es6RewriteClass")
268-
.setInternalFactory(Es6RewriteClass::new)
268+
.setInternalFactory(
269+
(compiler) ->
270+
new Es6RewriteClass(
271+
compiler, compiler.getOptions().getEs6SubclassTranspilation()))
269272
.build();
270273

271274
static final PassFactory getEs6RewriteDestructuring(ObjectDestructuringRewriteMode rewriteMode) {

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

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.google.javascript.jscomp.TypedScopeCreator.DYNAMIC_EXTENDS_WITHOUT_JSDOC;
2121
import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
2222

23+
import com.google.javascript.jscomp.CompilerOptions.Es6SubclassTranspilation;
2324
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
2425
import com.google.javascript.jscomp.colors.Color;
2526
import com.google.javascript.jscomp.colors.StandardColors;
@@ -47,6 +48,8 @@ public final class Es6RewriteClassTest extends CompilerTestCase {
4748
.addJSCompLibraries()
4849
.build();
4950

51+
private Es6SubclassTranspilation es6SubclassTranspilation;
52+
5053
public Es6RewriteClassTest() {
5154
super(EXTERNS_BASE);
5255
}
@@ -62,6 +65,7 @@ public void customSetUp() throws Exception {
6265
replaceTypesWithColors();
6366
enableMultistageCompilation();
6467
setGenericNameReplacements(Es6NormalizeClasses.GENERIC_NAME_REPLACEMENTS);
68+
es6SubclassTranspilation = Es6SubclassTranspilation.CONCISE_UNSAFE;
6569
}
6670

6771
private static PassFactory makePassFactory(
@@ -74,7 +78,9 @@ protected CompilerPass getProcessor(final Compiler compiler) {
7478
PhaseOptimizer optimizer = new PhaseOptimizer(compiler, null);
7579
optimizer.addOneTimePass(makePassFactory("es6NormalizeClasses", Es6NormalizeClasses::new));
7680
optimizer.addOneTimePass(makePassFactory("es6ConvertSuper", Es6ConvertSuper::new));
77-
optimizer.addOneTimePass(makePassFactory("es6RewriteClass", Es6RewriteClass::new));
81+
optimizer.addOneTimePass(
82+
makePassFactory(
83+
"es6RewriteClass", (c) -> new Es6RewriteClass(compiler, es6SubclassTranspilation)));
7884
return optimizer;
7985
}
8086

@@ -245,6 +251,29 @@ public void testAnonymousWithSuper() {
245251
""");
246252
}
247253

254+
@Test
255+
public void testAnonymousWithSuper_reflectConstruct() {
256+
es6SubclassTranspilation = Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT;
257+
enableClosurePass();
258+
ignoreWarnings(FunctionTypeBuilder.RESOLVED_TAG_EMPTY, TypeCheck.POSSIBLE_INEXISTENT_PROPERTY);
259+
test(
260+
"""
261+
goog.forwardDeclare('D')
262+
f(class extends D { f() { super.g() } })
263+
""",
264+
"""
265+
goog.forwardDeclare('D')
266+
/** @constructor
267+
*/
268+
const CLASS_DECL$0 = function() {
269+
return $jscomp.construct(D, arguments, this.constructor);
270+
};
271+
$jscomp.inherits(CLASS_DECL$0, D);
272+
CLASS_DECL$0.prototype.f = function() { D.prototype.g.call(this); };
273+
f(CLASS_DECL$0)
274+
""");
275+
}
276+
248277
@Test
249278
public void testComplexBaseWithConditional() {
250279
test(
@@ -622,6 +651,21 @@ public void testExtendsWithImplicitConstructorNoTypeInfo() {
622651
"""));
623652
}
624653

654+
@Test
655+
public void testExtendsWithImplicitConstructor_knownSuperclass_doesNotEs6SubclassTranspilation() {
656+
es6SubclassTranspilation = Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT;
657+
test(
658+
"class D {} class C extends D {}",
659+
"""
660+
/** @constructor */
661+
let D = function() {};
662+
/** @constructor
663+
*/
664+
let C = function() { D.apply(this, arguments); };
665+
$jscomp.inherits(C, D);
666+
""");
667+
}
668+
625669
@Test
626670
public void testExtendsWithSpreadsPassedToSuper() {
627671
test(
@@ -717,6 +761,31 @@ function f() {
717761
"""));
718762
}
719763

764+
@Test
765+
public void testExtends_nonGlobalClass_usingReflectConstruct() {
766+
es6SubclassTranspilation = Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT;
767+
test(
768+
srcs(
769+
"""
770+
function f() {
771+
class C {}
772+
class D extends C {}
773+
}
774+
"""),
775+
expected(
776+
"""
777+
function f() {
778+
/** @constructor */
779+
let C = function() {};
780+
/** @constructor */
781+
let D = function() {
782+
return $jscomp.construct(C, arguments, this.constructor);
783+
};
784+
$jscomp.inherits(D, C);
785+
}
786+
"""));
787+
}
788+
720789
@Test
721790
public void testExtendsExternsClass() {
722791
test(
@@ -1333,6 +1402,47 @@ class C extends D {
13331402
""");
13341403
}
13351404

1405+
@Test
1406+
public void testAlternativeSuperCalls_withUnknownSuperclass_es6SubclassTranspilation() {
1407+
// Class being extended is unknown, so we must assume super() could change the value of `this`.
1408+
enableClosurePass();
1409+
ignoreWarnings(FunctionTypeBuilder.RESOLVED_TAG_EMPTY);
1410+
es6SubclassTranspilation = Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT;
1411+
test(
1412+
"""
1413+
goog.forwardDeclare('D');
1414+
class C extends D {
1415+
/** @param {string} strArg
1416+
* @param {number} n */
1417+
constructor(strArg, n) {
1418+
if (n >= 0) {
1419+
super('positive: ' + strArg);
1420+
} else {
1421+
super('negative: ' + strArg);
1422+
}
1423+
this.n = n;
1424+
}
1425+
}
1426+
""",
1427+
"""
1428+
goog.forwardDeclare('D');
1429+
/** @constructor */
1430+
let C = function(strArg, n) {
1431+
var $jscomp$super$this$m1146332801$0;
1432+
if (n >= 0) {
1433+
$jscomp$super$this$m1146332801$0 =
1434+
$jscomp.construct(D, ["positive: " + strArg], this.constructor);
1435+
} else {
1436+
$jscomp$super$this$m1146332801$0 =
1437+
$jscomp.construct(D, ["negative: " + strArg], this.constructor);
1438+
}
1439+
$jscomp$super$this$m1146332801$0.n = n;
1440+
return $jscomp$super$this$m1146332801$0;
1441+
}
1442+
$jscomp.inherits(C, D);
1443+
""");
1444+
}
1445+
13361446
@Test
13371447
public void testComputedSuper() {
13381448
test(
@@ -2070,6 +2180,27 @@ public void testInheritFromExterns() {
20702180
"""));
20712181
}
20722182

2183+
@Test
2184+
public void testInheritFromExterns_withReflectConstruct() {
2185+
es6SubclassTranspilation = Es6SubclassTranspilation.SAFE_REFLECT_CONSTRUCT;
2186+
test(
2187+
externs(
2188+
EXTERNS_BASE
2189+
+ """
2190+
/** @constructor */ function ExternsClass() {}
2191+
ExternsClass.m = function() {};
2192+
"""),
2193+
srcs("class CodeClass extends ExternsClass {}"),
2194+
expected(
2195+
"""
2196+
/** @constructor */
2197+
let CodeClass = function() {
2198+
return $jscomp.construct(ExternsClass, arguments, this.constructor);
2199+
};
2200+
$jscomp.inherits(CodeClass,ExternsClass)
2201+
"""));
2202+
}
2203+
20732204
// Make sure we don't crash on this code.
20742205
// https://github.com/google/closure-compiler/issues/752
20752206
@Test

0 commit comments

Comments
 (0)