Skip to content
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

Fix: class transform should not drop method definition when key contains non-BMP characters #14897

Merged
merged 5 commits into from Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions packages/babel-helper-function-name/src/index.ts
Expand Up @@ -197,9 +197,22 @@ function visit(
}

/**
* @param {NodePath} param0
* @param {Boolean} localBinding whether a name could shadow a self-reference (e.g. converting arrow function)
* @param {Boolean} supportUnicodeId whether a target support unicodeId or not
* Add id to function/class expression inferred from the AST
*
* @export
* @template N The unamed expression type
* @param {Object} nodePathLike The NodePath-like input
* @param {N} nodePathLike.node an AST node
* @param {NodePath<N>["parent"]} [nodePathLike.parent] The parent of the AST node
* @param {Scope} nodePathLike.scope The scope associated to the AST node
* @param {t.LVal | t.StringLiteral | t.NumericLiteral | t.BigIntLiteral} [nodePathLike.id] the fallback naming source when the helper
* can not infer the function name from the AST
* @param {boolean} [localBinding=false] whether a name could shadow a self-reference (e.g. converting arrow function)
* @param {boolean} [supportUnicodeId=false] whether the compilation target supports unicodeId (non-BMP characters) or not
* @returns {(N | t.CallExpression | void)}
* - modified node when name can be inferred,
* - an IIFE when `node` contains a binding shadowing the inferred function name (e.g. `let f = function (f) {}`),
* - `void` when `node` has `id` property or the helper can not inferred the name or the inferred name contains non-BMP characters that is not supported by current target
*/
export default function <N extends t.FunctionExpression | t.Class>(
{
Expand All @@ -209,13 +222,13 @@ export default function <N extends t.FunctionExpression | t.Class>(
id,
}: {
node: N;
parent?: t.Node;
parent?: NodePath<N>["parent"];
scope: Scope;
id?: t.LVal | t.StringLiteral | t.NumericLiteral | t.BigIntLiteral;
},
localBinding = false,
supportUnicodeId = false,
): t.CallExpression | N {
): N | t.CallExpression | void {
// has an `id` so we don't need to infer one
if (node.id) return;

Expand Down
2 changes: 1 addition & 1 deletion packages/babel-helper-wrap-function/src/index.ts
Expand Up @@ -148,7 +148,7 @@ function plainFunction(
const returnFn = container.callee.body.body[1].argument;
nameFunction({
node: returnFn,
parent: path.parent,
parent: (path as NodePath<t.FunctionExpression>).parent,
scope: path.scope,
});
functionId = returnFn.id;
Expand Down
1 change: 1 addition & 0 deletions packages/babel-plugin-transform-classes/package.json
Expand Up @@ -15,6 +15,7 @@
"main": "./lib/index.js",
"dependencies": {
"@babel/helper-annotate-as-pure": "workspace:^",
"@babel/helper-compilation-targets": "workspace:^",
"@babel/helper-environment-visitor": "workspace:^",
"@babel/helper-function-name": "workspace:^",
"@babel/helper-optimise-call-expression": "workspace:^",
Expand Down
26 changes: 19 additions & 7 deletions packages/babel-plugin-transform-classes/src/index.ts
@@ -1,4 +1,5 @@
import { declare } from "@babel/helper-plugin-utils";
import { isRequired } from "@babel/helper-compilation-targets";
import annotateAsPure from "@babel/helper-annotate-as-pure";
import nameFunction from "@babel/helper-function-name";
import splitExportDeclaration from "@babel/helper-split-export-declaration";
Expand Down Expand Up @@ -30,6 +31,10 @@ export default declare((api, options: Options) => {
"superIsCallableConstructor",
) ?? loose) as boolean;
const noClassCalls = (api.assumption("noClassCalls") ?? loose) as boolean;
const supportUnicodeId = !isRequired(
"transform-unicode-escapes",
api.targets(),
);

// todo: investigate traversal requeueing
const VISITED = new WeakSet();
Expand Down Expand Up @@ -59,7 +64,7 @@ export default declare((api, options: Options) => {
const { node } = path;
if (VISITED.has(node)) return;

const inferred = nameFunction(path);
const inferred = nameFunction(path, undefined, supportUnicodeId);
if (inferred && inferred !== node) {
path.replaceWith(inferred);
return;
Expand All @@ -68,12 +73,19 @@ export default declare((api, options: Options) => {
VISITED.add(node);

const [replacedPath] = path.replaceWith(
transformClass(path, state.file, builtinClasses, loose, {
setClassMethods,
constantSuper,
superIsCallableConstructor,
noClassCalls,
}),
transformClass(
path,
state.file,
builtinClasses,
loose,
{
setClassMethods,
constantSuper,
superIsCallableConstructor,
noClassCalls,
},
supportUnicodeId,
),
);

if (replacedPath.isCallExpression()) {
Expand Down
26 changes: 20 additions & 6 deletions packages/babel-plugin-transform-classes/src/transformClass.ts
Expand Up @@ -98,6 +98,7 @@ export default function transformClass(
builtinClasses: ReadonlySet<string>,
isLoose: boolean,
assumptions: ClassAssumptions,
supportUnicodeId: boolean,
) {
const classState: State = {
parent: undefined,
Expand Down Expand Up @@ -508,7 +509,14 @@ export default function transformClass(
if (node.kind === "method") {
// @ts-expect-error Fixme: we are passing a ClassMethod to nameFunction, but nameFunction
// does not seem to support it
fn = nameFunction({ id: key, node: node, scope });
fn =
nameFunction(
// @ts-expect-error Fixme: we are passing a ClassMethod to nameFunction, but nameFunction
// does not seem to support it
{ id: key, node: node, scope },
undefined,
supportUnicodeId,
) ?? fn;
}
} else {
// todo(flow->ts) find a way to avoid "key as t.StringLiteral" below which relies on this assignment
Expand Down Expand Up @@ -570,11 +578,17 @@ export default function transformClass(

const key = t.toComputedKey(node, node.key);
if (t.isStringLiteral(key)) {
func = nameFunction({
node: func,
id: key,
scope,
});
// @ts-expect-error: requires strictNullCheck
func =
nameFunction(
{
node: func,
id: key,
scope,
},
undefined,
supportUnicodeId,
) ?? func;
}

const expr = t.expressionStatement(
Expand Down
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,4 @@
{
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

var _proto = o.prototype;

_proto.𠮷野家 = function () {};

return babelHelpers.createClass(o);
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,4 @@
{
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

var _proto = o.prototype;

_proto.𠮷野家 = function () {};

return babelHelpers.createClass(o);
}();
@@ -0,0 +1,4 @@
{
"targets": "chrome 44",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

var _proto = o.prototype;

_proto.𠮷野家 = function 𠮷野家() {};

return babelHelpers.createClass(o);
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": false,
"targets": "chrome 44",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "\uD842\uDFB7\u91CE\u5BB6",
value: function 𠮷野家() {}
Comment on lines +8 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current main it is transformed to

  babelHelpers.createClass(o, [{
    key: "\uD842\uDFB7\u91CE\u5BB6"
  }]

here the value is dropped because nameFunction returns undefined.

}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": false,
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "\uD842\uDFB7\u91CE\u5BB6",
value: function () {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": true,
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "𠮷野家",
value: function () {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": true,
"targets": "chrome 44",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "𠮷野家",
value: function 𠮷野家() {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": false,
"targets": "chrome 44",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "\uD842\uDFB7\u91CE\u5BB6",
value: function 𠮷野家() {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": false,
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "\uD842\uDFB7\u91CE\u5BB6",
value: function () {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": true,
"targets": "chrome 43",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "𠮷野家",
value: function () {}
}]);
return o;
}();
@@ -0,0 +1 @@
var o = class { 𠮷野家() {} };
@@ -0,0 +1,5 @@
{
"BABEL_8_BREAKING": true,
"targets": "chrome 44",
"plugins": ["transform-classes"]
}
@@ -0,0 +1,13 @@
var o = /*#__PURE__*/function () {
"use strict";

function o() {
babelHelpers.classCallCheck(this, o);
}

babelHelpers.createClass(o, [{
key: "𠮷野家",
value: function 𠮷野家() {}
}]);
return o;
}();