Skip to content

Commit

Permalink
Chrome pass: Don't keep the shorthand name of a class when rewriting …
Browse files Browse the repository at this point in the history
…cr.define calls.

It's unnecessary because all references already get updated to the new name, and it simplifies the transpilation if we drop it.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159048748
  • Loading branch information
tbreisacher committed Jun 15, 2017
1 parent 0bbeaf3 commit 52a7ea7
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/com/google/javascript/jscomp/ChromePass.java
Expand Up @@ -433,6 +433,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
// by looking up in this.exports for internalName to find the correspondent
// externalName.
Node clone = n.cloneTree();
if (clone.isClass()) {
Node className = clone.getFirstChild();
className.replaceWith(IR.empty().useSourceInfoFrom(className));
}
NodeUtil.markNewScopesChanged(clone, compiler);
Node exprResult =
IR.exprResult(IR.assign(buildQualifiedName(n.getFirstChild()), clone).srcref(n))
Expand Down
42 changes: 40 additions & 2 deletions test/com/google/javascript/jscomp/ChromePassTest.java
Expand Up @@ -400,7 +400,7 @@ public void testCrDefineFunction() throws Exception {
"});"));
}

public void testCrDefineClass() throws Exception {
public void testCrDefineClassStatement() throws Exception {
test(
LINE_JOINER.join(
"cr.define('settings', function() {",
Expand All @@ -412,7 +412,45 @@ public void testCrDefineClass() throws Exception {
"var settings = settings || {};",
"cr.define('settings', function() {",
" var x = 0;",
" settings.C = class C {}",
" settings.C = class {}",
" return { C: settings.C };",
"});"));
}

public void testCrDefineClassExpression() throws Exception {
test(
LINE_JOINER.join(
"cr.define('settings', function() {",
" var x = 0;",
" var C = class {}",
" return { C: C };",
"});"),
LINE_JOINER.join(
"var settings = settings || {};",
"cr.define('settings', function() {",
" var x = 0;",
" settings.C = class {}",
" return { C: settings.C };",
"});"));
}

public void testCrDefineClassWithInternalSelfReference() throws Exception {
test(
LINE_JOINER.join(
"cr.define('settings', function() {",
" var x = 0;",
" class C {",
" static create() { return new C; }",
" }",
" return { C: C };",
"});"),
LINE_JOINER.join(
"var settings = settings || {};",
"cr.define('settings', function() {",
" var x = 0;",
" settings.C = class {",
" static create() { return new settings.C; }",
" }",
" return { C: settings.C };",
"});"));
}
Expand Down
5 changes: 2 additions & 3 deletions test/com/google/javascript/jscomp/IntegrationTest.java
Expand Up @@ -462,7 +462,7 @@ public void testChromePass_noTranspile() {
"var my = my || {};",
"my.namespace = my.namespace || {};",
"cr.define('my.namespace', function() {",
" my.namespace.X = class X {};",
" my.namespace.X = class {};",
" return { X: my.namespace.X };",
"});"));
}
Expand All @@ -480,8 +480,7 @@ public void testChromePass_transpile() {
"my.namespace = my.namespace || {};",
"cr.define('my.namespace', function() {",
" /** @constructor */",
" var i0$classdecl$var0 = function() {};",
" my.namespace.X = i0$classdecl$var0;",
" my.namespace.X = function() {};",
" return { X: my.namespace.X };",
"});"));
}
Expand Down

0 comments on commit 52a7ea7

Please sign in to comment.