New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't insert __self: this
within constructors of derived classes (#13550)
#13552
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47527/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e277fc5:
|
__self: this
prior to super()
calls (#13550)__self: this
within constructors that contain super()
calls (#13550)
__self: this
within constructors that contain super()
calls (#13550)__self: this
within constructors of derived classes (#13550)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also have a new JSX transform (https://it.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html), and it also needs to be patched.
You can test it with this config:
{
plugins: [
["@babel/plugin-transform-react-jsx/lib/development, { runtime: "automatic" }]
],
}
And with this input code:
class A extends B {
constructor() {
<div />;
super();
}
}
it produces this:
var _jsxFileName = "";
import { jsxDEV as _jsxDEV } from "react/jsx-dev-runtime";
class A extends B {
constructor() {
/*#__PURE__*/
_jsxDEV("div", {}, void 0, false, {
fileName: _jsxFileName,
lineNumber: 4,
columnNumber: 5
}, this);
super();
}
}
@nicolo-ribaudo I was not able to find a place to put the utility methods ( |
So... What is the next step for moving this forward? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not able to find a place to put the utility methods (isThisAllowed, etc.) so I just duplicated the same code. Other than that, I think it is working now.
The old plugin will eventually fade away in the future, so this duplication should be ok.
Could you rebase this branch on main
? It should make CI pass.
`__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.
…ve a `super()` call.
Ok I did that. Hopefully I didn't botch it... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
…abel#13552) * Don't insert `__self: this` prior to `super()` calls (babel#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
In plugin the "babel-plugin-transform-react-jsx-self",
__self: this
is inserted for debugging purposes. However, it will cause a runtime error if it is inserted prior to asuper()
call in a constructor. This PR will prevent__self: this
from inserted when inside a constructor of a derived class.