From c26de18886b448a97a5e12073e8552804bee1de3 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 14 May 2018 21:04:45 +0200 Subject: [PATCH 1/4] fix: handle non array statements in evaluate helper --- .../babel-helper-evaluate-path/src/index.js | 27 ++++++++++--------- .../__tests__/minify-env-tests.js | 15 +++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 3e887edc9..751d4d88f 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,7 +1,5 @@ "use strict"; -const t = require("@babel/types"); - module.exports = function evaluate(path, { tdz = false } = {}) { if (!tdz && !path.isReferencedIdentifier()) { return baseEvaluate(path); @@ -122,14 +120,15 @@ function evaluateBasedOnControlFlow(binding, refPath) { // early-exit const declaration = binding.path.parentPath; + if (declaration.parentPath.removed) { + return { confident: true, value: void 0 }; + } + if ( - t.isIfStatement(declaration.parentPath) || - t.isLoop(declaration.parentPath) || - t.isSwitchCase(declaration.parentPath) + declaration.parentPath.isIfStatement() || + declaration.parentPath.isLoop() || + declaration.parentPath.isSwitchCase() ) { - if (declaration.parentPath.removed) { - return { confident: true, value: void 0 }; - } return { shouldDeopt: true }; } @@ -181,9 +180,10 @@ function evaluateBasedOnControlFlow(binding, refPath) { const declaration = declarator.parentPath; if ( - declaration.parentPath.isIfStatement() || - declaration.parentPath.isLoop() || - declaration.parentPath.isSwitchCase() + declaration.parentPath && + (declaration.parentPath.isIfStatement() || + declaration.parentPath.isLoop() || + declaration.parentPath.isSwitchCase()) ) { return { shouldDeopt: true }; } @@ -194,7 +194,10 @@ function evaluateBasedOnControlFlow(binding, refPath) { } // Detect Usage before Init - const stmts = scopePath.get("body"); + let stmts = scopePath.get("body"); + if (!Array.isArray(stmts)) { + stmts = [stmts]; + } const compareResult = compareBindingAndReference({ binding, diff --git a/packages/babel-preset-minify/__tests__/minify-env-tests.js b/packages/babel-preset-minify/__tests__/minify-env-tests.js index 99c6036ea..003ec25ef 100644 --- a/packages/babel-preset-minify/__tests__/minify-env-tests.js +++ b/packages/babel-preset-minify/__tests__/minify-env-tests.js @@ -251,4 +251,19 @@ describe("preset along with env", () => { } ` ); + thePlugin( + "should fix issue#844", + ` + class MyComponent { + } + MyComponent.propTypes = { + userName: 123 + } + `, + ` + "use strict"; + class a {} + a.propTypes = { userName: 123 }; + ` + ); }); From 221a0753dd18f2528cee64b5a475ecd750287274 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 14 May 2018 21:26:30 +0200 Subject: [PATCH 2/4] add babel-types deps --- .../babel-helper-evaluate-path/package.json | 5 ++++- .../babel-helper-evaluate-path/src/index.js | 22 +++++++++---------- .../__tests__/fixtures/issue-844/actual.js | 2 ++ .../__tests__/fixtures/issue-844/expected.js | 5 +++++ .../__tests__/minify-env-tests.js | 15 ------------- yarn.lock | 2 +- 6 files changed, 23 insertions(+), 28 deletions(-) create mode 100644 packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/actual.js create mode 100644 packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/expected.js diff --git a/packages/babel-helper-evaluate-path/package.json b/packages/babel-helper-evaluate-path/package.json index 43c90182e..d6b24d825 100644 --- a/packages/babel-helper-evaluate-path/package.json +++ b/packages/babel-helper-evaluate-path/package.json @@ -11,5 +11,8 @@ "license": "MIT", "author": "boopathi", "main": "lib/index.js", - "repository": "https://github.com/babel/minify/tree/master/packages/babel-helper-evaluate-path" + "repository": "https://github.com/babel/minify/tree/master/packages/babel-helper-evaluate-path", + "dependencies": { + "@babel/types": "^7.0.0-beta.46" + } } diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 751d4d88f..8b723144b 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,5 +1,7 @@ "use strict"; +const t = require("@babel/types"); + module.exports = function evaluate(path, { tdz = false } = {}) { if (!tdz && !path.isReferencedIdentifier()) { return baseEvaluate(path); @@ -120,15 +122,14 @@ function evaluateBasedOnControlFlow(binding, refPath) { // early-exit const declaration = binding.path.parentPath; - if (declaration.parentPath.removed) { - return { confident: true, value: void 0 }; - } - if ( - declaration.parentPath.isIfStatement() || - declaration.parentPath.isLoop() || - declaration.parentPath.isSwitchCase() + t.isIfStatement(declaration.parentPath) || + t.isLoop(declaration.parentPath) || + t.isSwitchCase(declaration.parentPath) ) { + if (declaration.parentPath.removed) { + return { confident: true, value: void 0 }; + } return { shouldDeopt: true }; } @@ -180,10 +181,9 @@ function evaluateBasedOnControlFlow(binding, refPath) { const declaration = declarator.parentPath; if ( - declaration.parentPath && - (declaration.parentPath.isIfStatement() || - declaration.parentPath.isLoop() || - declaration.parentPath.isSwitchCase()) + t.isIfStatement(declaration.parentPath) || + t.isLoop(declaration.parentPath) || + t.isSwitchCase(declaration.parentPath) ) { return { shouldDeopt: true }; } diff --git a/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/actual.js b/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/actual.js new file mode 100644 index 000000000..270c51440 --- /dev/null +++ b/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/actual.js @@ -0,0 +1,2 @@ +class MyComponent {} +MyComponent.propTypes = { userName: 123 }; diff --git a/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/expected.js b/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/expected.js new file mode 100644 index 000000000..96c1a00d1 --- /dev/null +++ b/packages/babel-plugin-minify-constant-folding/__tests__/fixtures/issue-844/expected.js @@ -0,0 +1,5 @@ +class MyComponent {} + +MyComponent.propTypes = { + userName: 123 +}; \ No newline at end of file diff --git a/packages/babel-preset-minify/__tests__/minify-env-tests.js b/packages/babel-preset-minify/__tests__/minify-env-tests.js index 003ec25ef..99c6036ea 100644 --- a/packages/babel-preset-minify/__tests__/minify-env-tests.js +++ b/packages/babel-preset-minify/__tests__/minify-env-tests.js @@ -251,19 +251,4 @@ describe("preset along with env", () => { } ` ); - thePlugin( - "should fix issue#844", - ` - class MyComponent { - } - MyComponent.propTypes = { - userName: 123 - } - `, - ` - "use strict"; - class a {} - a.propTypes = { userName: 123 }; - ` - ); }); diff --git a/yarn.lock b/yarn.lock index a2fcbaa14..41a90d83b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -504,7 +504,7 @@ invariant "^2.2.0" lodash "^4.2.0" -"@babel/types@7.0.0-beta.46": +"@babel/types@7.0.0-beta.46", "@babel/types@^7.0.0-beta.46": version "7.0.0-beta.46" resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.0.0-beta.46.tgz#eb84399a699af9fcb244440cce78e1acbeb40e0c" dependencies: From 91bfeb203fc65b1244a385484fbb480533411d55 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 14 May 2018 22:44:07 +0200 Subject: [PATCH 3/4] use path.is check --- packages/babel-helper-evaluate-path/src/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 8b723144b..361dded1b 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -181,9 +181,10 @@ function evaluateBasedOnControlFlow(binding, refPath) { const declaration = declarator.parentPath; if ( - t.isIfStatement(declaration.parentPath) || - t.isLoop(declaration.parentPath) || - t.isSwitchCase(declaration.parentPath) + declaration.parentPath && + (declaration.parentPath.isIfStatement() || + declaration.parentPath.isLoop() || + declaration.parentPath.isSwitchCase()) ) { return { shouldDeopt: true }; } From a8112a5f6b9bb50378207eb0f3a418f9a998864b Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Mon, 14 May 2018 23:04:13 +0200 Subject: [PATCH 4/4] remove babel-types dep --- .../babel-helper-evaluate-path/package.json | 5 +---- .../babel-helper-evaluate-path/src/index.js | 20 +++++++++++-------- yarn.lock | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/babel-helper-evaluate-path/package.json b/packages/babel-helper-evaluate-path/package.json index d6b24d825..43c90182e 100644 --- a/packages/babel-helper-evaluate-path/package.json +++ b/packages/babel-helper-evaluate-path/package.json @@ -11,8 +11,5 @@ "license": "MIT", "author": "boopathi", "main": "lib/index.js", - "repository": "https://github.com/babel/minify/tree/master/packages/babel-helper-evaluate-path", - "dependencies": { - "@babel/types": "^7.0.0-beta.46" - } + "repository": "https://github.com/babel/minify/tree/master/packages/babel-helper-evaluate-path" } diff --git a/packages/babel-helper-evaluate-path/src/index.js b/packages/babel-helper-evaluate-path/src/index.js index 361dded1b..e8b13e656 100644 --- a/packages/babel-helper-evaluate-path/src/index.js +++ b/packages/babel-helper-evaluate-path/src/index.js @@ -1,7 +1,5 @@ "use strict"; -const t = require("@babel/types"); - module.exports = function evaluate(path, { tdz = false } = {}) { if (!tdz && !path.isReferencedIdentifier()) { return baseEvaluate(path); @@ -122,14 +120,20 @@ function evaluateBasedOnControlFlow(binding, refPath) { // early-exit const declaration = binding.path.parentPath; + /** + * Handle when binding is created inside a parent block and + * the corresponding parent is removed by other plugins + * if (false) { var a } -> var a + */ + if (declaration.parentPath && declaration.parentPath.removed) { + return { confident: true, value: void 0 }; + } + if ( - t.isIfStatement(declaration.parentPath) || - t.isLoop(declaration.parentPath) || - t.isSwitchCase(declaration.parentPath) + declaration.parentPath.isIfStatement() || + declaration.parentPath.isLoop() || + declaration.parentPath.isSwitchCase() ) { - if (declaration.parentPath.removed) { - return { confident: true, value: void 0 }; - } return { shouldDeopt: true }; } diff --git a/yarn.lock b/yarn.lock index 41a90d83b..a2fcbaa14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -504,7 +504,7 @@ invariant "^2.2.0" lodash "^4.2.0" -"@babel/types@7.0.0-beta.46", "@babel/types@^7.0.0-beta.46": +"@babel/types@7.0.0-beta.46": version "7.0.0-beta.46" resolved "https://registry.yarnpkg.com/@babel/types/-/types-7.0.0-beta.46.tgz#eb84399a699af9fcb244440cce78e1acbeb40e0c" dependencies: