From 3fc904e1d4674effa0b948b454ce8789f5d4c089 Mon Sep 17 00:00:00 2001 From: Siddhant N Trivedi Date: Wed, 12 Feb 2020 16:25:38 +0530 Subject: [PATCH] Fix classNameTDZError in computed prototype methods with class fields (#11068) * Draft fix for TDZError in computed prototype methods * Added tests for loose:false and also extracted handleClassTDZ * Added state parameter to handleClassTDZ function * Extracted the state object to a variable * Added helper comments for environmentVisitor * Added else condition to traverse computed Path * Cached computed path key into a variable --- .../src/misc.js | 53 ++++++++++--------- .../loose-false/input.js | 6 +++ .../loose-false/options.json | 10 ++++ .../loose-false/output.js | 10 ++++ .../loose-true/input.js | 6 +++ .../loose-true/options.json | 10 ++++ .../loose-true/output.js | 8 +++ .../babel-helper-replace-supers/src/index.js | 1 + 8 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/input.js 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 create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/options.json create mode 100644 packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/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..31f8e0948676 100644 --- a/packages/babel-helper-create-class-features-plugin/src/misc.js +++ b/packages/babel-helper-create-class-features-plugin/src/misc.js @@ -25,26 +25,24 @@ const referenceVisitor = { } }, }; +function handleClassTDZ(path, state) { + if ( + state.classBinding && + state.classBinding === path.scope.getBinding(path.node.name) + ) { + const classNameTDZError = state.file.addHelper("classNameTDZError"); + const throwNode = t.callExpression(classNameTDZError, [ + t.stringLiteral(path.node.name), + ]); + + path.replaceWith(t.sequenceExpression([throwNode, path.node])); + path.skip(); + } +} -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(); - } - }, - }, - environmentVisitor, -]); +const classFieldDefinitionEvaluationTDZVisitor = { + ReferencedIdentifier: handleClassTDZ, +}; export function injectInitialization(path, constructor, nodes, renamer) { if (!nodes.length) return; @@ -84,17 +82,22 @@ 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) { - computedPath.traverse(classFieldDefinitionEvaluationTDZVisitor, { - classBinding: path.node.id && path.scope.getBinding(path.node.id.name), - file, - }); + const computedKey = computedPath.get("key"); + if (computedKey.isReferencedIdentifier()) { + handleClassTDZ(computedKey, state); + } else { + 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, ); diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/input.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-false/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-false/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/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/tdz-error-computed-prototypes/loose-true/options.json b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/options.json new file mode 100644 index 000000000000..0a3e4189c2b1 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/options.json @@ -0,0 +1,10 @@ +{ + "plugins": [ + [ + "proposal-class-properties", + { + "loose": true + } + ] + ] +} diff --git a/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/output.js b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/output.js new file mode 100644 index 000000000000..c42939ebe066 --- /dev/null +++ b/packages/babel-helper-create-class-features-plugin/test/fixtures/tdz-error-computed-prototypes/loose-true/output.js @@ -0,0 +1,8 @@ +class Foo { + ['HELLO']() { + console.log('>>>>', Foo); + } + +} + +Foo.nickname = 'Tom'; diff --git a/packages/babel-helper-replace-supers/src/index.js b/packages/babel-helper-replace-supers/src/index.js index 72d1e9849b07..8f117a7a6b70 100644 --- a/packages/babel-helper-replace-supers/src/index.js +++ b/packages/babel-helper-replace-supers/src/index.js @@ -41,6 +41,7 @@ function skipAllButComputedKey(path: NodePath) { } } +// environmentVisitor should be used when traversing the whole class and not for specific class elements/methods. export const environmentVisitor = { TypeAnnotation(path: NodePath) { path.skip();