Don't insert __self: this within constructors of derived classes (#13552)

* Don't insert `__self: this` prior to `super()` calls (#13550)

`__self: this` is inserted for debugging purposes. However, it will cause a runtime error if it is inserted prior to a `super()` call in a constructor. This commit will prevent `__self: this` from inserted when there is a following `super()` call.

* Prevent adding `__self` within a constructor that has `super()` altogether.

* Fix 2 typos in the comments.

* Add an additional test case for constructors that do not have a `super()` call.

* Detect `super()` call by testing whether the class has a superclass.

* Update method name and corresponding comments

* Add an additional test for the case where the derived class do not have a `super()` call.

* Apply the same changes to babel-plugin-transform-react-jsx
This commit is contained in:
Rin Tepis 2021-07-27 01:32:29 +08:00 committed by GitHub
parent 4a56387330
commit 224a35c5c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 570 additions and 13 deletions

View File

@ -0,0 +1,51 @@
class A { }
class B extends A {
constructor() {
<sometag1 />;
super(<sometag2 />);
<sometag3 />;
}
}
class C {
constructor() {
<sometag4 />;
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = () => <sometag5 />;
this.y = function () {
return <sometag6 />;
};
function z() {
return <sometag7 />;
}
{ <sometag8 /> }
super();
}
}
class F {
constructor() {
<sometag9 />
}
}
class G extends A {
constructor() {
return <sometag10 />;
}
}

View File

@ -0,0 +1,118 @@
var _reactJsxDevRuntime = require("react/jsx-dev-runtime");
var _jsxFileName = "<CWD>/packages/babel-plugin-transform-react-jsx-development/test/fixtures/linux/within-derived-classes-constructor/input.js";
class A {}
class B extends A {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag1", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 5,
columnNumber: 5
}, void 0);
super( /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag2", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 6,
columnNumber: 11
}, void 0));
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag3", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 7,
columnNumber: 5
}, void 0);
}
}
class C {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag4", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 13,
columnNumber: 5
}, this);
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = function () {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag5", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 29,
columnNumber: 20
}, void 0);
};
this.y = function () {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag6", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 31,
columnNumber: 14
}, this);
};
function z() {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag7", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 34,
columnNumber: 14
}, this);
}
{
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag8", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 36,
columnNumber: 7
}, void 0);
}
super();
}
}
class F {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag9", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 43,
columnNumber: 5
}, this);
}
}
class G extends A {
constructor() {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag10", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 49,
columnNumber: 12
}, void 0);
}
}

View File

@ -0,0 +1,51 @@
class A { }
class B extends A {
constructor() {
<sometag1 />;
super(<sometag2 />);
<sometag3 />;
}
}
class C {
constructor() {
<sometag4 />;
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = () => <sometag5 />;
this.y = function () {
return <sometag6 />;
};
function z() {
return <sometag7 />;
}
{ <sometag8 /> }
super();
}
}
class F {
constructor() {
<sometag9 />
}
}
class G extends A {
constructor() {
return <sometag10 />;
}
}

View File

@ -0,0 +1,118 @@
var _reactJsxDevRuntime = require("react/jsx-dev-runtime");
var _jsxFileName = "<CWD>\\packages\\babel-plugin-transform-react-jsx-development\\test\\fixtures\\windows\\within-derived-classes-constructor-windows\\input.js";
class A {}
class B extends A {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag1", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 5,
columnNumber: 5
}, void 0);
super( /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag2", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 6,
columnNumber: 11
}, void 0));
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag3", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 7,
columnNumber: 5
}, void 0);
}
}
class C {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag4", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 13,
columnNumber: 5
}, this);
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = function () {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag5", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 29,
columnNumber: 20
}, void 0);
};
this.y = function () {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag6", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 31,
columnNumber: 14
}, this);
};
function z() {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag7", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 34,
columnNumber: 14
}, this);
}
{
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag8", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 36,
columnNumber: 7
}, void 0);
}
super();
}
}
class F {
constructor() {
/*#__PURE__*/
_reactJsxDevRuntime.jsxDEV("sometag9", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 43,
columnNumber: 5
}, this);
}
}
class G extends A {
constructor() {
return /*#__PURE__*/_reactJsxDevRuntime.jsxDEV("sometag10", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 49,
columnNumber: 12
}, void 0);
}
}

View File

@ -1,7 +1,8 @@
/** /**
* This adds a __self={this} JSX attribute to all JSX elements, which React will use * This adds a __self={this} JSX attribute to all JSX elements, which React will use
* to generate some runtime warnings. * to generate some runtime warnings. However, if the JSX element appears within a
* * constructor of a derived class, `__self={this}` will not be inserted in order to
* prevent runtime errors.
* *
* == JSX Literals == * == JSX Literals ==
* *
@ -16,11 +17,64 @@ import { types as t } from "@babel/core";
const TRACE_ID = "__self"; const TRACE_ID = "__self";
/**
* Finds the closest parent function that provides `this`. Specifically, this looks for
* the first parent function that isn't an arrow function.
*
* Derived from `Scope#getFunctionParent`
*/
function getThisFunctionParent(path) {
let scope = path.scope;
do {
if (
scope.path.isFunctionParent() &&
!scope.path.isArrowFunctionExpression()
) {
return scope.path;
}
} while ((scope = scope.parent));
return null;
}
/**
* Returns whether the class has specified a superclass.
*/
function isDerivedClass(classPath) {
return classPath.node.superClass !== null;
}
/**
* Returns whether `this` is allowed at given path.
*/
function isThisAllowed(path) {
// This specifically skips arrow functions as they do not rewrite `this`.
const parentMethodOrFunction = getThisFunctionParent(path);
if (parentMethodOrFunction === null) {
// We are not in a method or function. It is fine to use `this`.
return true;
}
if (!parentMethodOrFunction.isMethod()) {
// If the closest parent is a regular function, `this` will be rebound, therefore it is fine to use `this`.
return true;
}
// Current node is within a method, so we need to check if the method is a constructor.
if (parentMethodOrFunction.node.kind !== "constructor") {
// We are not in a constructor, therefore it is always fine to use `this`.
return true;
}
// Now we are in a constructor. If it is a derived class, we do not reference `this`.
return !isDerivedClass(parentMethodOrFunction.parentPath.parentPath);
}
export default declare(api => { export default declare(api => {
api.assertVersion(7); api.assertVersion(7);
const visitor = { const visitor = {
JSXOpeningElement({ node }) { JSXOpeningElement(path) {
if (!isThisAllowed(path)) {
return;
}
const node = path.node;
const id = t.jsxIdentifier(TRACE_ID); const id = t.jsxIdentifier(TRACE_ID);
const trace = t.thisExpression(); const trace = t.thisExpression();

View File

@ -0,0 +1,51 @@
class A { }
class B extends A {
constructor() {
<sometag1 />;
super(<sometag2 />);
<sometag3 />;
}
}
class C {
constructor() {
<sometag4 />;
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = () => <sometag5 />;
this.y = function () {
return <sometag6 />;
};
function z() {
return <sometag7 />;
}
{ <sometag8 /> }
super();
}
}
class F {
constructor() {
<sometag9 />
}
}
class G extends A {
constructor() {
return <sometag10 />;
}
}

View File

@ -0,0 +1,65 @@
class A {}
class B extends A {
constructor() {
<sometag1 />;
super(<sometag2 />);
<sometag3 />;
}
}
class C {
constructor() {
<sometag4 __self={this} />;
class D extends A {
constructor() {
super();
}
}
const E = class extends A {
constructor() {
super();
}
};
}
}
class E extends A {
constructor() {
this.x = () => <sometag5 />;
this.y = function () {
return <sometag6 __self={this} />;
};
function z() {
return <sometag7 __self={this} />;
}
{
<sometag8 />;
}
super();
}
}
class F {
constructor() {
<sometag9 __self={this} />;
}
}
class G extends A {
constructor() {
return <sometag10 />;
}
}

View File

@ -103,16 +103,22 @@ export default function createPlugin({ name, development }) {
} }
} }
const self = t.jsxAttribute( const attributes = [];
t.jsxIdentifier("__self"), if (isThisAllowed(path)) {
t.jsxExpressionContainer(t.thisExpression()), attributes.push(
t.jsxAttribute(
t.jsxIdentifier("__self"),
t.jsxExpressionContainer(t.thisExpression()),
),
);
}
attributes.push(
t.jsxAttribute(
t.jsxIdentifier("__source"),
t.jsxExpressionContainer(makeSource(path, state)),
),
); );
const source = t.jsxAttribute( path.pushContainer("attributes", attributes);
t.jsxIdentifier("__source"),
t.jsxExpressionContainer(makeSource(path, state)),
);
path.pushContainer("attributes", [self, source]);
}, },
}; };
@ -277,6 +283,49 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
}, },
}; };
// Finds the closest parent function that provides `this`. Specifically, this looks for
// the first parent function that isn't an arrow function.
//
// Derived from `Scope#getFunctionParent`
function getThisFunctionParent(path) {
let scope = path.scope;
do {
if (
scope.path.isFunctionParent() &&
!scope.path.isArrowFunctionExpression()
) {
return scope.path;
}
} while ((scope = scope.parent));
return null;
}
// Returns whether the class has specified a superclass.
function isDerivedClass(classPath) {
return classPath.node.superClass !== null;
}
// Returns whether `this` is allowed at given path.
function isThisAllowed(path) {
// This specifically skips arrow functions as they do not rewrite `this`.
const parentMethodOrFunction = getThisFunctionParent(path);
if (parentMethodOrFunction === null) {
// We are not in a method or function. It is fine to use `this`.
return true;
}
if (!parentMethodOrFunction.isMethod()) {
// If the closest parent is a regular function, `this` will be rebound, therefore it is fine to use `this`.
return true;
}
// Current node is within a method, so we need to check if the method is a constructor.
if (parentMethodOrFunction.node.kind !== "constructor") {
// We are not in a constructor, therefore it is always fine to use `this`.
return true;
}
// Now we are in a constructor. If it is a derived class, we do not reference `this`.
return !isDerivedClass(parentMethodOrFunction.parentPath.parentPath);
}
function call(pass, name, args) { function call(pass, name, args) {
const node = t.callExpression(get(pass, `id/${name}`)(), args); const node = t.callExpression(get(pass, `id/${name}`)(), args);
if (PURE_ANNOTATION ?? get(pass, "defaultPure")) annotateAsPure(node); if (PURE_ANNOTATION ?? get(pass, "defaultPure")) annotateAsPure(node);
@ -476,7 +525,7 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
extracted.key ?? path.scope.buildUndefinedNode(), extracted.key ?? path.scope.buildUndefinedNode(),
t.booleanLiteral(children.length > 1), t.booleanLiteral(children.length > 1),
extracted.__source ?? path.scope.buildUndefinedNode(), extracted.__source ?? path.scope.buildUndefinedNode(),
extracted.__self ?? t.thisExpression(), extracted.__self ?? path.scope.buildUndefinedNode(),
); );
} else if (extracted.key !== undefined) { } else if (extracted.key !== undefined) {
args.push(extracted.key); args.push(extracted.key);