From f39811d27118dc3c4236a1ce44679601597a92d2 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Fri, 1 Sep 2017 17:14:25 -0400 Subject: [PATCH] Derived constructors don't always need a super (#6189) --- .../src/vanilla.js | 38 +++++++++---------- .../actual.js | 5 +++ .../options.json | 4 ++ .../actual.js | 5 +++ .../expected.js | 11 ++++++ .../actual.js | 5 +++ .../expected.js | 11 ++++++ .../actual.js | 5 +++ .../expected.js | 12 ++++++ .../actual.js | 5 +++ .../expected.js | 12 ++++++ 11 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/actual.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/options.json create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/actual.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/expected.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/actual.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/expected.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/actual.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/expected.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/actual.js create mode 100644 packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/expected.js diff --git a/packages/babel-plugin-transform-es2015-classes/src/vanilla.js b/packages/babel-plugin-transform-es2015-classes/src/vanilla.js index 442dc2e37c..0b94bd1131 100644 --- a/packages/babel-plugin-transform-es2015-classes/src/vanilla.js +++ b/packages/babel-plugin-transform-es2015-classes/src/vanilla.js @@ -97,7 +97,6 @@ export default class ClassTransformer { this.pushedConstructor = false; this.pushedInherits = false; - this.pushedThis = false; this.isLoose = false; this.superThises = []; @@ -267,12 +266,6 @@ export default class ClassTransformer { if (isConstructor) { path.traverse(verifyConstructorVisitor, this); - - if (!this.hasBareSuper && this.isDerived) { - throw path.buildCodeFrameError( - "missing super() call in constructor", - ); - } } const replaceSupers = new ReplaceSupers( @@ -440,17 +433,12 @@ export default class ClassTransformer { // turn it into a return if (this.superThises.length) { - bareSuper.scope.push({ id: thisRef }); - call = t.assignmentExpression("=", thisRef, call); + call = t.assignmentExpression("=", thisRef(), call); } bareSuper.parentPath.replaceWith(t.returnStatement(call)); } else { - if (!this.pushedThis) { - body.scope.push({ id: thisRef }); - this.pushedThis = true; - } - bareSuper.replaceWith(t.assignmentExpression("=", thisRef, call)); + bareSuper.replaceWith(t.assignmentExpression("=", thisRef(), call)); } } @@ -460,12 +448,20 @@ export default class ClassTransformer { const path = this.userConstructorPath; const body = path.get("body"); + if (!this.hasBareSuper && !this.superReturns.length) { + throw path.buildCodeFrameError("missing super() call in constructor"); + } + path.traverse(findThisesVisitor, this); let guaranteedSuperBeforeFinish = !!this.bareSupers.length; const superRef = this.superName || t.identifier("Function"); - const thisRef = path.scope.generateUidIdentifier("this"); + let thisRef = function() { + const ref = path.scope.generateDeclaredUidIdentifier("this"); + thisRef = () => ref; + return ref; + }; for (const bareSuper of this.bareSupers) { this.wrapSuperCall(bareSuper, superRef, thisRef, body); @@ -486,7 +482,7 @@ export default class ClassTransformer { } for (const thisPath of this.superThises) { - thisPath.replaceWith(thisRef); + thisPath.replaceWith(thisRef()); } let wrapReturn; @@ -494,14 +490,14 @@ export default class ClassTransformer { if (this.isLoose) { wrapReturn = returnArg => { return returnArg - ? t.logicalExpression("||", returnArg, thisRef) - : thisRef; + ? t.logicalExpression("||", returnArg, thisRef()) + : thisRef(); }; } else { wrapReturn = returnArg => t.callExpression( this.file.addHelper("possibleConstructorReturn"), - [thisRef].concat(returnArg || []), + [thisRef()].concat(returnArg || []), ); } @@ -511,7 +507,9 @@ export default class ClassTransformer { if (bodyPaths.length && !bodyPaths.pop().isReturnStatement()) { body.pushContainer( "body", - t.returnStatement(guaranteedSuperBeforeFinish ? thisRef : wrapReturn()), + t.returnStatement( + guaranteedSuperBeforeFinish ? thisRef() : wrapReturn(), + ), ); } diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/actual.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/actual.js new file mode 100644 index 0000000000..5fae7a7b14 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/actual.js @@ -0,0 +1,5 @@ +class Foo extends Bar { + constructor() { + + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/options.json b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/options.json new file mode 100644 index 0000000000..9d624ae5d2 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-must-call-super/options.json @@ -0,0 +1,4 @@ +{ + "throws": "missing super() call in constructor", + "plugins": ["transform-es2015-classes"] +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/actual.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/actual.js new file mode 100644 index 0000000000..b4ddfdde03 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/actual.js @@ -0,0 +1,5 @@ +class Child extends Base { + constructor(){ + return false; + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/expected.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/expected.js new file mode 100644 index 0000000000..028df6cacb --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-falsey/expected.js @@ -0,0 +1,11 @@ +var Child = function (_Base) { + babelHelpers.inheritsLoose(Child, _Base); + + function Child() { + var _this; + + return false || _this; + } + + return Child; +}(Base); diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/actual.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/actual.js new file mode 100644 index 0000000000..2ea3cbbcc9 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/actual.js @@ -0,0 +1,5 @@ +class Child extends Base { + constructor(){ + return {}; + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/expected.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/expected.js new file mode 100644 index 0000000000..eb7583c880 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/loose/derived-constructor-no-super-return-object/expected.js @@ -0,0 +1,11 @@ +var Child = function (_Base) { + babelHelpers.inheritsLoose(Child, _Base); + + function Child() { + var _this; + + return {} || _this; + } + + return Child; +}(Base); diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/actual.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/actual.js new file mode 100644 index 0000000000..e4f97282d1 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/actual.js @@ -0,0 +1,5 @@ +class Child extends Base { + constructor(){ + return false; + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/expected.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/expected.js new file mode 100644 index 0000000000..6e7ae545a9 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-falsey/expected.js @@ -0,0 +1,12 @@ +var Child = function (_Base) { + babelHelpers.inherits(Child, _Base); + + function Child() { + var _this; + + babelHelpers.classCallCheck(this, Child); + return babelHelpers.possibleConstructorReturn(_this, false); + } + + return Child; +}(Base); diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/actual.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/actual.js new file mode 100644 index 0000000000..51f6eb8e51 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/actual.js @@ -0,0 +1,5 @@ +class Child extends Base { + constructor(){ + return {}; + } +} diff --git a/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/expected.js b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/expected.js new file mode 100644 index 0000000000..13cd4a1e97 --- /dev/null +++ b/packages/babel-plugin-transform-es2015-classes/test/fixtures/spec/derived-constructor-no-super-return-object/expected.js @@ -0,0 +1,12 @@ +var Child = function (_Base) { + babelHelpers.inherits(Child, _Base); + + function Child() { + var _this; + + babelHelpers.classCallCheck(this, Child); + return babelHelpers.possibleConstructorReturn(_this, {}); + } + + return Child; +}(Base);