From 263404dad83dc9998f00e2cbb99bafdf26145ea3 Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Thu, 30 Jan 2020 04:07:13 +0530 Subject: [PATCH 1/7] Draft fix for TDZError in computed prototype methods --- .../src/misc.js | 48 ++++++++++--------- .../tdz-error-computed-prototypes/input.js | 6 +++ .../options.json | 10 ++++ .../tdz-error-computed-prototypes/output.js | 8 ++++ 4 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index ff5178374a56..ccb15a108eb5 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -26,25 +26,22 @@ const referenceVisitor = { }, }; -const classFieldDefinitionEvaluationTDZVisitor = traverse.visitors.merge([ - { - ReferencedIdentifier(path) { - if ( - this.classBinding && - this.classBinding === path.scope.getBinding(path.node.name) - ) { - const classNameTDZError = this.file.addHelper("classNameTDZError"); - const throwNode = t.callExpression(classNameTDZError, [ - t.stringLiteral(path.node.name), - ]); - - path.replaceWith(t.sequenceExpression([throwNode, path.node])); - path.skip(); - } - }, +const classFieldDefinitionEvaluationTDZVisitor = { + ReferencedIdentifier(path) { + if ( + this.classBinding && + this.classBinding === path.scope.getBinding(path.node.name) + ) { + const classNameTDZError = this.file.addHelper("classNameTDZError"); + const throwNode = t.callExpression(classNameTDZError, [ + t.stringLiteral(path.node.name), + ]); + + path.replaceWith(t.sequenceExpression([throwNode, path.node])); + path.skip(); + } }, - environmentVisitor, -]); +}; export function injectInitialization(path, constructor, nodes, renamer) { if (!nodes.length) return; @@ -86,10 +83,17 @@ export function extractComputedKeys(ref, path, computedPaths, file) { const declarations = []; for (const computedPath of computedPaths) { - computedPath.traverse(classFieldDefinitionEvaluationTDZVisitor, { - classBinding: path.node.id && path.scope.getBinding(path.node.id.name), - file, - }); + if (computedPath.get("key").isReferencedIdentifier()) { + classFieldDefinitionEvaluationTDZVisitor.ReferencedIdentifier.call( + computedPath.get("key"), + { + classBinding: + path.node.id && path.scope.getBinding(path.node.id.name), + file, + }, + ); + } + computedPath.get("key").traverse(classFieldDefinitionEvaluationTDZVisitor); const computedNode = computedPath.node; // Make sure computed property names are only evaluated once (upon class definition) diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js new file mode 100644 index 000000000000..cbebe00211cc --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js @@ -0,0 +1,6 @@ +class Foo { + static nickname = 'Tom'; + ['HELLO']() { + console.log('>>>>', Foo); + } +} \ No newline at end of file diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json new file mode 100644 index 000000000000..0a3e4189c2b1 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json @@ -0,0 +1,10 @@ +{ + "plugins": [ + [ + "proposal-class-properties", + { + "loose": true + } + ] + ] +} diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js new file mode 100644 index 000000000000..c42939ebe066 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js @@ -0,0 +1,8 @@ +class Foo { + ['HELLO']() { + console.log('>>>>', Foo); + } + +} + +Foo.nickname = 'Tom'; From 287b5d8322fea131290a79e0320e55b65749c7f6 Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Thu, 30 Jan 2020 05:23:51 +0530 Subject: [PATCH 2/7] Added tests for loose:false and also extracted handleClassTDZ --- .../src/misc.js | 47 +++++++++---------- .../loose-false}/input.js | 0 .../loose-false/options.json | 10 ++++ .../loose-false/output.js | 10 ++++ .../loose-true/input.js | 6 +++ .../loose-true}/options.json | 0 .../loose-true}/output.js | 0 7 files changed, 49 insertions(+), 24 deletions(-) rename packages/babel-helper-create-class-features-plugin/test/fixtures/{plugin-proposal-class-properties/tdz-error-computed-prototypes => tdz-error-computed-prototypes/loose-false}/input.js (100%) create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/options.json create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/output.js create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/input.js rename packages/babel-helper-create-class-features-plugin/test/fixtures/{plugin-proposal-class-properties/tdz-error-computed-prototypes => tdz-error-computed-prototypes/loose-true}/options.json (100%) rename packages/babel-helper-create-class-features-plugin/test/fixtures/{plugin-proposal-class-properties/tdz-error-computed-prototypes => tdz-error-computed-prototypes/loose-true}/output.js (100%) diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index ccb15a108eb5..c41e646bbdd1 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -25,22 +25,23 @@ const referenceVisitor = { } }, }; +function handleClassTDZ(path) { + if ( + this.classBinding && + this.classBinding === path.scope.getBinding(path.node.name) + ) { + const classNameTDZError = this.file.addHelper("classNameTDZError"); + const throwNode = t.callExpression(classNameTDZError, [ + t.stringLiteral(path.node.name), + ]); + + path.replaceWith(t.sequenceExpression([throwNode, path.node])); + path.skip(); + } +} const classFieldDefinitionEvaluationTDZVisitor = { - ReferencedIdentifier(path) { - if ( - this.classBinding && - this.classBinding === path.scope.getBinding(path.node.name) - ) { - const classNameTDZError = this.file.addHelper("classNameTDZError"); - const throwNode = t.callExpression(classNameTDZError, [ - t.stringLiteral(path.node.name), - ]); - - path.replaceWith(t.sequenceExpression([throwNode, path.node])); - path.skip(); - } - }, + ReferencedIdentifier: handleClassTDZ, }; export function injectInitialization(path, constructor, nodes, renamer) { @@ -81,19 +82,17 @@ export function injectInitialization(path, constructor, nodes, renamer) { export function extractComputedKeys(ref, path, computedPaths, file) { const declarations = []; - for (const computedPath of computedPaths) { if (computedPath.get("key").isReferencedIdentifier()) { - classFieldDefinitionEvaluationTDZVisitor.ReferencedIdentifier.call( - computedPath.get("key"), - { - classBinding: - path.node.id && path.scope.getBinding(path.node.id.name), - file, - }, - ); + handleClassTDZ(computedPath.get("key"), { + classBinding: path.node.id && path.scope.getBinding(path.node.id.name), + file, + }); } - computedPath.get("key").traverse(classFieldDefinitionEvaluationTDZVisitor); + computedPath.get("key").traverse(classFieldDefinitionEvaluationTDZVisitor, { + classBinding: path.node.id && path.scope.getBinding(path.node.id.name), + file, + }); const computedNode = computedPath.node; // Make sure computed property names are only evaluated once (upon class definition) diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/input.js similarity index 100% rename from packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/input.js rename to packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/input.js diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/options.json b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/options.json new file mode 100644 index 000000000000..4a570ff574c8 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/options.json @@ -0,0 +1,10 @@ +{ + "plugins": [ + [ + "proposal-class-properties", + { + "loose": false + } + ] + ] +} diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/output.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/output.js new file mode 100644 index 000000000000..b4a5e95cfb40 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/output.js @@ -0,0 +1,10 @@ +function _defineProperty(obj, key, value) { if (key in obj) { Object.defineProperty(obj, key, { value: value, enumerable: true, configurable: true, writable: true }); } else { obj[key] = value; } return obj; } + +class Foo { + ['HELLO']() { + console.log('>>>>', Foo); + } + +} + +_defineProperty(Foo, "nickname", 'Tom'); diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/input.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/input.js new file mode 100644 index 000000000000..cbebe00211cc --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/input.js @@ -0,0 +1,6 @@ +class Foo { + static nickname = 'Tom'; + ['HELLO']() { + console.log('>>>>', Foo); + } +} \ No newline at end of file diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/options.json similarity index 100% rename from packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/options.json rename to packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/options.json diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/output.js similarity index 100% rename from packages/babel-helper-create-class-features-plugin/test/fixtures/plugin-proposal-class-properties/tdz-error-computed-prototypes/output.js rename to packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/output.js From fa7927f31fe1ec1203ca601b1638dd1745add7ef Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Thu, 30 Jan 2020 05:28:51 +0530 Subject: [PATCH 3/7] Added state parameter to handleClassTDZ function --- .../babel-helper-create-class-features-plugin/src/misc.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index c41e646bbdd1..54c05772287a 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -25,12 +25,12 @@ const referenceVisitor = { } }, }; -function handleClassTDZ(path) { +function handleClassTDZ(path, state) { if ( - this.classBinding && - this.classBinding === path.scope.getBinding(path.node.name) + state.classBinding && + state.classBinding === path.scope.getBinding(path.node.name) ) { - const classNameTDZError = this.file.addHelper("classNameTDZError"); + const classNameTDZError = state.file.addHelper("classNameTDZError"); const throwNode = t.callExpression(classNameTDZError, [ t.stringLiteral(path.node.name), ]); From 5b0a2e06ae2ea1f6b7f752046d7caeaafa6f716a Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Thu, 30 Jan 2020 22:49:16 +0530 Subject: [PATCH 4/7] Extracted the state object to a variable --- .../src/misc.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index 54c05772287a..4b40e013dd6b 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -82,17 +82,17 @@ export function injectInitialization(path, constructor, nodes, renamer) { export function extractComputedKeys(ref, path, computedPaths, file) { const declarations = []; + const state = { + classBinding: path.node.id && path.scope.getBinding(path.node.id.name), + file, + }; for (const computedPath of computedPaths) { if (computedPath.get("key").isReferencedIdentifier()) { - handleClassTDZ(computedPath.get("key"), { - classBinding: path.node.id && path.scope.getBinding(path.node.id.name), - file, - }); + handleClassTDZ(computedPath.get("key"), state); } - computedPath.get("key").traverse(classFieldDefinitionEvaluationTDZVisitor, { - classBinding: path.node.id && path.scope.getBinding(path.node.id.name), - file, - }); + computedPath + .get("key") + .traverse(classFieldDefinitionEvaluationTDZVisitor, state); const computedNode = computedPath.node; // Make sure computed property names are only evaluated once (upon class definition) From a883e3487bf045b03bfeab307a32a9800459fb03 Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Sat, 1 Feb 2020 00:47:56 +0530 Subject: [PATCH 5/7] Added helper comments for environmentVisitor --- packages/babel-helper-replace-supers/src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/babel-helper-replace-supers/src/index.js b/packages/babel-helper-replace-supers/src/index.js index 3412d321caf2..f990d10a09d9 100644 --- a/packages/babel-helper-replace-supers/src/index.js +++ b/packages/babel-helper-replace-supers/src/index.js @@ -40,6 +40,7 @@ function skipAllButComputedKey(path) { } } +// environmentVisitor should be used when traversing the whole class and not for specific class elements/methods. export const environmentVisitor = { TypeAnnotation(path) { path.skip(); From c3572f45242dae9a05f0f6de83812092f238f37c Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Sat, 1 Feb 2020 01:42:11 +0530 Subject: [PATCH 6/7] Added else condition to traverse computed Path --- .../babel-helper-create-class-features-plugin/src/misc.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index 4b40e013dd6b..4248faf89e68 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -89,10 +89,11 @@ export function extractComputedKeys(ref, path, computedPaths, file) { for (const computedPath of computedPaths) { if (computedPath.get("key").isReferencedIdentifier()) { handleClassTDZ(computedPath.get("key"), state); + } else { + computedPath + .get("key") + .traverse(classFieldDefinitionEvaluationTDZVisitor, state); } - computedPath - .get("key") - .traverse(classFieldDefinitionEvaluationTDZVisitor, state); const computedNode = computedPath.node; // Make sure computed property names are only evaluated once (upon class definition) From 0d70fbe76f77d26fac2ecbbef2d9636599fd5667 Mon Sep 17 00:00:00 2001 From: sidntrivedi012 Date: Sun, 2 Feb 2020 02:36:03 +0530 Subject: [PATCH 7/7] Cached computed path key into a variable --- .../src/misc.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.js b/packages/babel-helper-create-class-features-plugin/src/misc.js index 4248faf89e68..31f8e0948676 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -87,18 +87,17 @@ export function extractComputedKeys(ref, path, computedPaths, file) { file, }; for (const computedPath of computedPaths) { - if (computedPath.get("key").isReferencedIdentifier()) { - handleClassTDZ(computedPath.get("key"), state); + const computedKey = computedPath.get("key"); + if (computedKey.isReferencedIdentifier()) { + handleClassTDZ(computedKey, state); } else { - computedPath - .get("key") - .traverse(classFieldDefinitionEvaluationTDZVisitor, state); + computedKey.traverse(classFieldDefinitionEvaluationTDZVisitor, state); } const computedNode = computedPath.node; // Make sure computed property names are only evaluated once (upon class definition) // and in the right order in combination with static properties - if (!computedPath.get("key").isConstantExpression()) { + if (!computedKey.isConstantExpression()) { const ident = path.scope.generateUidIdentifierBasedOnNode( computedNode.key, );