From e6153dce62c8e2466e6a4832e25d09579f2d3322 Mon Sep 17 00:00:00 2001 From: YeonjuHwang Date: Sun, 6 Jun 2021 20:26:43 +0900 Subject: [PATCH 1/6] fix: reference to class expression in private method --- .../src/fields.ts | 18 +++++++++- .../src/index.ts | 7 ++-- .../private/static-self-field/exec.js | 19 ++++++++++ .../private/static-self-field/input.js | 8 +++++ .../private/static-self-field/options.json | 3 ++ .../private/static-self-field/output.js | 10 ++++++ .../private/static-self-method/exec.js | 35 +++++++++++++++++++ .../private/static-self-method/input.js | 22 ++++++++++++ .../private/static-self-method/options.json | 3 ++ .../private/static-self-method/output.js | 25 +++++++++++++ 10 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/options.json create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/options.json create mode 100644 packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts index 908d0b133062..f629fda2c064 100644 --- a/packages/babel-helper-create-class-features-plugin/src/fields.ts +++ b/packages/babel-helper-create-class-features-plugin/src/fields.ts @@ -669,6 +669,16 @@ const thisContextVisitor = traverse.visitors.merge([ environmentVisitor, ]); +const innerReferencesVisitor = { + ReferencedIdentifier(path, state) { + const { name } = path.node; + if (path.scope.bindingIdentifierEquals(name, state.innerBinding)) { + state.needsClassRef = true; + path.replaceWith(t.cloneNode(state.classRef)); + } + }, +}; + function replaceThisContext( path, ref, @@ -677,7 +687,8 @@ function replaceThisContext( isStaticBlock, constantSuper, ) { - const state = { classRef: ref, needsClassRef: false }; + const { innerBinding, ...classRef } = ref; + const state = { classRef, needsClassRef: false, innerBinding }; const replacer = new ReplaceSupers({ methodPath: path, @@ -696,6 +707,11 @@ function replaceThisContext( if (isStaticBlock || path.isProperty()) { path.traverse(thisContextVisitor, state); } + + if (state.classRef?.name !== innerBinding?.name) { + path.traverse(innerReferencesVisitor, state); + } + return state.needsClassRef; } diff --git a/packages/babel-helper-create-class-features-plugin/src/index.ts b/packages/babel-helper-create-class-features-plugin/src/index.ts index e071aa2845c9..7ec57d3f6f0f 100644 --- a/packages/babel-helper-create-class-features-plugin/src/index.ts +++ b/packages/babel-helper-create-class-features-plugin/src/index.ts @@ -168,8 +168,7 @@ export function createClassFeaturePlugin({ if (!props.length && !isDecorated) return; - let ref; - + let ref = undefined; if (path.isClassExpression() || !path.node.id) { nameFunction(path); ref = path.scope.generateUidIdentifier("class"); @@ -177,6 +176,10 @@ export function createClassFeaturePlugin({ ref = t.cloneNode(path.node.id); } + if (ref !== undefined) { + ref.innerBinding = path.node.id; + } + // NODE: These three functions don't support decorators yet, // but verifyUsedFeatures throws if there are both // decorators and private fields. diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js new file mode 100644 index 000000000000..e9eb5fca1c60 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js @@ -0,0 +1,19 @@ +function wrapper(wc) { + return wc +} + +const f = wrapper(class Foo { + static #x = Foo; + static y = Foo; + + static extract() { + return { + x: Foo.#x, + y: Foo.y, + } + } +}); + +const { x, y } = f.extract(); +expect(x).toBe(f) +expect(y).toBe(f) diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js new file mode 100644 index 000000000000..63d79fa51602 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js @@ -0,0 +1,8 @@ +function wrapper(wc) { + return wc +} + +const f = wrapper(class Foo { + static #x = Foo; + static y = Foo; +}); diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/options.json b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/options.json new file mode 100644 index 000000000000..c13414457ab2 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["proposal-class-properties", "transform-block-scoping"] +} diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js new file mode 100644 index 000000000000..e8691f4651b8 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js @@ -0,0 +1,10 @@ +var _class, _temp, _x; + +function wrapper(wc) { + return wc; +} + +var f = wrapper((_temp = _class = class Foo {}, _x = { + writable: true, + value: _class +}, babelHelpers.defineProperty(_class, "y", _class), _temp)); diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js new file mode 100644 index 000000000000..e8c687de3448 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js @@ -0,0 +1,35 @@ +function wrapper(wc) { + return wc +} + +const f = wrapper(class Foo { + static #bar() { + return Foo; + } + + static #method() { + return function inner() { + return Foo; + }; + } + static #method_shadowed() { + new Foo(); + return function inner() { + let Foo = 3; + return Foo; + } + } + + static extract() { + return { + bar: Foo.#bar, + method: Foo.#method, + method_shadowed: Foo.#method_shadowed + } + } +}); + +const { bar, method, method_shadowed } = f.extract(); +expect(bar()).toBe(f) +expect(method()()).toBe(f) +expect(method_shadowed()()).toBe(3) diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js new file mode 100644 index 000000000000..800fffe2a2a4 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js @@ -0,0 +1,22 @@ +function wrapper(wc) { + return wc +} + +wrapper(class Foo { + static #bar() { + return Foo; + } + + static #method() { + return function inner() { + return Foo; + }; + } + static #method_shadowed() { + new Foo(); + return function inner() { + let Foo = 3; + return Foo; + } + } +}); diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/options.json b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/options.json new file mode 100644 index 000000000000..c13414457ab2 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["proposal-class-properties", "transform-block-scoping"] +} diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js new file mode 100644 index 000000000000..a87c55e1fae3 --- /dev/null +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js @@ -0,0 +1,25 @@ +var _class; + +function wrapper(wc) { + return wc; +} + +wrapper(_class = class Foo {}); + +function _bar() { + return _class; +} + +function _method() { + return function inner() { + return _class; + }; +} + +function _method_shadowed() { + new _class(); + return function inner() { + var Foo = 3; + return Foo; + }; +} From 9f9f5f937e8f68c681c1d2a8008c5bf03543ebee Mon Sep 17 00:00:00 2001 From: YeonjuHwang Date: Thu, 10 Jun 2021 01:35:21 +0900 Subject: [PATCH 2/6] modify path.node.name instead of requeueing the AST node remove wrapper function on test which is no need split innerBinding from ref because ref is AST node. --- .../src/fields.ts | 14 ++++++++++---- .../src/index.ts | 8 +++----- .../fixtures/private/static-self-field/exec.js | 8 ++------ .../fixtures/private/static-self-field/input.js | 8 ++------ .../fixtures/private/static-self-field/output.js | 8 ++------ 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts index f629fda2c064..bf94598ab151 100644 --- a/packages/babel-helper-create-class-features-plugin/src/fields.ts +++ b/packages/babel-helper-create-class-features-plugin/src/fields.ts @@ -674,7 +674,7 @@ const innerReferencesVisitor = { const { name } = path.node; if (path.scope.bindingIdentifierEquals(name, state.innerBinding)) { state.needsClassRef = true; - path.replaceWith(t.cloneNode(state.classRef)); + path.node.name = state.classRef.name; } }, }; @@ -686,9 +686,13 @@ function replaceThisContext( file, isStaticBlock, constantSuper, + innerBindingRef, ) { - const { innerBinding, ...classRef } = ref; - const state = { classRef, needsClassRef: false, innerBinding }; + const state = { + classRef: ref, + needsClassRef: false, + innerBinding: innerBindingRef, + }; const replacer = new ReplaceSupers({ methodPath: path, @@ -708,7 +712,7 @@ function replaceThisContext( path.traverse(thisContextVisitor, state); } - if (state.classRef?.name !== innerBinding?.name) { + if (state.classRef?.name && state.classRef.name !== innerBindingRef?.name) { path.traverse(innerReferencesVisitor, state); } @@ -724,6 +728,7 @@ export function buildFieldsInitNodes( setPublicClassFields, privateFieldsAsProperties, constantSuper, + innerBindingRef, ) { let needsClassRef = false; let injectSuperRef; @@ -759,6 +764,7 @@ export function buildFieldsInitNodes( state, isStaticBlock, constantSuper, + innerBindingRef, ); needsClassRef = needsClassRef || replaced; } diff --git a/packages/babel-helper-create-class-features-plugin/src/index.ts b/packages/babel-helper-create-class-features-plugin/src/index.ts index 7ec57d3f6f0f..48fd826867e7 100644 --- a/packages/babel-helper-create-class-features-plugin/src/index.ts +++ b/packages/babel-helper-create-class-features-plugin/src/index.ts @@ -168,7 +168,8 @@ export function createClassFeaturePlugin({ if (!props.length && !isDecorated) return; - let ref = undefined; + const innerBinding = path.node.id; + let ref; if (path.isClassExpression() || !path.node.id) { nameFunction(path); ref = path.scope.generateUidIdentifier("class"); @@ -176,10 +177,6 @@ export function createClassFeaturePlugin({ ref = t.cloneNode(path.node.id); } - if (ref !== undefined) { - ref.innerBinding = path.node.id; - } - // NODE: These three functions don't support decorators yet, // but verifyUsedFeatures throws if there are both // decorators and private fields. @@ -223,6 +220,7 @@ export function createClassFeaturePlugin({ setPublicClassFields ?? loose, privateFieldsAsProperties ?? loose, constantSuper ?? loose, + innerBinding, )); } diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js index e9eb5fca1c60..388fed537f7d 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/exec.js @@ -1,8 +1,4 @@ -function wrapper(wc) { - return wc -} - -const f = wrapper(class Foo { +const f = class Foo { static #x = Foo; static y = Foo; @@ -12,7 +8,7 @@ const f = wrapper(class Foo { y: Foo.y, } } -}); +}; const { x, y } = f.extract(); expect(x).toBe(f) diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js index 63d79fa51602..24e0bf9e918d 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/input.js @@ -1,8 +1,4 @@ -function wrapper(wc) { - return wc -} - -const f = wrapper(class Foo { +const f = class Foo { static #x = Foo; static y = Foo; -}); +} \ No newline at end of file diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js index e8691f4651b8..e24d4e7024e4 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-field/output.js @@ -1,10 +1,6 @@ var _class, _temp, _x; -function wrapper(wc) { - return wc; -} - -var f = wrapper((_temp = _class = class Foo {}, _x = { +var f = (_temp = _class = class Foo {}, _x = { writable: true, value: _class -}, babelHelpers.defineProperty(_class, "y", _class), _temp)); +}, babelHelpers.defineProperty(_class, "y", _class), _temp); From ff9be6c1492e7027b2803666905c3a7198b37cb9 Mon Sep 17 00:00:00 2001 From: YeonjuHwang Date: Thu, 10 Jun 2021 07:53:52 +0900 Subject: [PATCH 3/6] remove wrapper function which is no need --- .../test/fixtures/private/static-self-method/exec.js | 8 ++------ .../test/fixtures/private/static-self-method/input.js | 8 ++------ .../test/fixtures/private/static-self-method/output.js | 6 +----- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js index e8c687de3448..09bb2297db3c 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/exec.js @@ -1,8 +1,4 @@ -function wrapper(wc) { - return wc -} - -const f = wrapper(class Foo { +const f = class Foo { static #bar() { return Foo; } @@ -27,7 +23,7 @@ const f = wrapper(class Foo { method_shadowed: Foo.#method_shadowed } } -}); +}; const { bar, method, method_shadowed } = f.extract(); expect(bar()).toBe(f) diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js index 800fffe2a2a4..a8c80e401bef 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/input.js @@ -1,8 +1,4 @@ -function wrapper(wc) { - return wc -} - -wrapper(class Foo { +const f = class Foo { static #bar() { return Foo; } @@ -19,4 +15,4 @@ wrapper(class Foo { return Foo; } } -}); +} diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js index a87c55e1fae3..b3f7c4cf2a5b 100644 --- a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js +++ b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/static-self-method/output.js @@ -1,10 +1,6 @@ var _class; -function wrapper(wc) { - return wc; -} - -wrapper(_class = class Foo {}); +var f = _class = class Foo {}; function _bar() { return _class; From 48313eb3c5e3536db3ed33c79a70b881f8f98cd1 Mon Sep 17 00:00:00 2001 From: Lively Date: Sat, 12 Jun 2021 12:14:09 +0900 Subject: [PATCH 4/6] Apply suggestions from code review apply feedbacks Co-authored-by: Henry Zhu --- .../babel-helper-create-class-features-plugin/src/fields.ts | 2 +- packages/babel-helper-create-class-features-plugin/src/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts index bf94598ab151..fd0f5fca8db0 100644 --- a/packages/babel-helper-create-class-features-plugin/src/fields.ts +++ b/packages/babel-helper-create-class-features-plugin/src/fields.ts @@ -672,7 +672,7 @@ const thisContextVisitor = traverse.visitors.merge([ const innerReferencesVisitor = { ReferencedIdentifier(path, state) { const { name } = path.node; - if (path.scope.bindingIdentifierEquals(name, state.innerBinding)) { + if (path.scope.bindingIdentifierEquals(path.node.name, state.innerBinding)) { state.needsClassRef = true; path.node.name = state.classRef.name; } diff --git a/packages/babel-helper-create-class-features-plugin/src/index.ts b/packages/babel-helper-create-class-features-plugin/src/index.ts index 48fd826867e7..52188266b931 100644 --- a/packages/babel-helper-create-class-features-plugin/src/index.ts +++ b/packages/babel-helper-create-class-features-plugin/src/index.ts @@ -170,7 +170,7 @@ export function createClassFeaturePlugin({ const innerBinding = path.node.id; let ref; - if (path.isClassExpression() || !path.node.id) { + if (path.isClassExpression() || !innerBinding) { nameFunction(path); ref = path.scope.generateUidIdentifier("class"); } else { From 5bd56869a3f744b8c5561bc580e0297b22b045b8 Mon Sep 17 00:00:00 2001 From: Lively Date: Mon, 14 Jun 2021 18:21:44 +0900 Subject: [PATCH 5/6] Update packages/babel-helper-create-class-features-plugin/src/index.ts Co-authored-by: Federico Ciardi --- packages/babel-helper-create-class-features-plugin/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/index.ts b/packages/babel-helper-create-class-features-plugin/src/index.ts index 52188266b931..359085b52b2b 100644 --- a/packages/babel-helper-create-class-features-plugin/src/index.ts +++ b/packages/babel-helper-create-class-features-plugin/src/index.ts @@ -170,7 +170,7 @@ export function createClassFeaturePlugin({ const innerBinding = path.node.id; let ref; - if (path.isClassExpression() || !innerBinding) { + if (!innerBinding || path.isClassExpression()) { nameFunction(path); ref = path.scope.generateUidIdentifier("class"); } else { From fad6be15455f160619410167cc3d8c41a47bb3d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 14 Jun 2021 16:52:54 +0200 Subject: [PATCH 6/6] Fix lint --- .../babel-helper-create-class-features-plugin/src/fields.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts index fd0f5fca8db0..29e94d746ef9 100644 --- a/packages/babel-helper-create-class-features-plugin/src/fields.ts +++ b/packages/babel-helper-create-class-features-plugin/src/fields.ts @@ -671,8 +671,9 @@ const thisContextVisitor = traverse.visitors.merge([ const innerReferencesVisitor = { ReferencedIdentifier(path, state) { - const { name } = path.node; - if (path.scope.bindingIdentifierEquals(path.node.name, state.innerBinding)) { + if ( + path.scope.bindingIdentifierEquals(path.node.name, state.innerBinding) + ) { state.needsClassRef = true; path.node.name = state.classRef.name; }