From f23e8cf5f6c3d3094a3a7a03a2ad6e4fbb7ece3b Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Fri, 4 Aug 2017 16:08:21 -0700 Subject: [PATCH 1/5] refactor mark declaration, add __PURE__ comments --- packages/regenerator-transform/src/visit.js | 56 +++++++++++---------- test/tests.transform.js | 54 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 26 deletions(-) diff --git a/packages/regenerator-transform/src/visit.js b/packages/regenerator-transform/src/visit.js index 6184c10f9..ef7d27139 100644 --- a/packages/regenerator-transform/src/visit.js +++ b/packages/regenerator-transform/src/visit.js @@ -153,6 +153,7 @@ exports.visitor = { if (wasGeneratorFunction && t.isExpression(node)) { util.replaceWithOrRemove(path, t.callExpression(util.runtimeProperty("mark"), [node])) + path.addComment("leading", "#__PURE__"); } // Generators are processed in 'exit' handlers so that regenerator only has to run on @@ -188,18 +189,34 @@ function getOuterFnExpr(funPath) { } let markDecl = getRuntimeMarkDecl(pp); - let markedArray = markDecl.declarations[0].id; - let funDeclIdArray = markDecl.declarations[0].init.callee.object; - t.assertArrayExpression(funDeclIdArray); - - let index = funDeclIdArray.elements.length; - funDeclIdArray.elements.push(node.id); - - return t.memberExpression( - markedArray, - t.numericLiteral(index), - true - ); + let declarations = markDecl.declarations; + let index = declarations.length; + + // this can change from pass to pass if something else is altering our body + // so we have to look it up every time :/ + let declarationsIndex = pp.node.body.findIndex((p) => { + return t.isVariableDeclaration(p) && p.declarations === declarations; + }); + assert(!isNaN(declarationsIndex)); + + // get a new unique id for our marked variable + let markedId = pp.scope.generateUidIdentifier("marked"); + + // now push our new declaration into our list + declarations.push(t.variableDeclarator( + markedId, + t.callExpression(util.runtimeProperty("mark"), [node.id]) + )); + + // assemble the path to our mark declarations + let bodyPathName = 'body.' + String(declarationsIndex) + '.declarations.' + String(index) + '.init'; + let bodyPath = pp.get(bodyPathName); + + // mark our call to util.runtimeProperty("mark") as pure to enable tree-shaking + bodyPath.addComment("leading", "#__PURE__"); + + // and return our new id + return markedId; } return node.id; @@ -214,20 +231,7 @@ function getRuntimeMarkDecl(blockPath) { return info.decl; } - info.decl = t.variableDeclaration("var", [ - t.variableDeclarator( - blockPath.scope.generateUidIdentifier("marked"), - t.callExpression( - t.memberExpression( - t.arrayExpression([]), - t.identifier("map"), - false - ), - [util.runtimeProperty("mark")] - ) - ) - ]); - + info.decl = t.variableDeclaration("var", []) blockPath.unshiftContainer("body", info.decl); return info.decl; diff --git a/test/tests.transform.js b/test/tests.transform.js index fbea408d5..cbaa8baf0 100644 --- a/test/tests.transform.js +++ b/test/tests.transform.js @@ -47,3 +47,57 @@ describe("_blockHoist nodes", function() { assert.deepEqual(names, ["hoistMe", "doNotHoistMe", "oyez"]); }); }); + +describe("marked", function() { + it("should work with a single function", function() { + var ast = recast.parse('function* foo(){};', { + parser: require("babylon") + }); + const code = recast.print(transform(ast)).code; + assert.notStrictEqual( + code.indexOf( + 'var _marked = regeneratorRuntime.mark(foo);' + ), -1 + ); + }); + it("should work with multiple functions", function() { + var ast = recast.parse([ + 'function* foo(){};', + 'function* bar() {};' + ].join("\n"), { + parser: require("babylon") + }); + const code = recast.print(transform(ast)).code; + assert.notStrictEqual( + code.indexOf( + 'var _marked = regeneratorRuntime.mark(foo), _marked2 = regeneratorRuntime.mark(bar);' + ), -1 + ); + }); +}); + +// TODO - how do i get recast to return comments? +describe("pure", function() { + xit("should work with a function expression", function() { + var ast = recast.parse('var a = function* foo(){};', { + parser: require("babylon") + }); + const code = recast.print(transform(ast)).code; + assert.notStrictEqual( + code.indexOf( + 'var a = /*#__PURE__*/regeneratorRuntime.mark(function foo() {' + ), -1 + ); + }); + xit("should work with a function declaration", function() { + var ast = recast.parse('function* foo(){};', { + parser: require("babylon") + }); + const code = recast.print(transform(ast)).code; + assert.notStrictEqual( + code.indexOf( + 'var a = /*#__PURE__*/Object.defineProperty( /*#__PURE__*/regeneratorRuntime.mark' + ), -1 + ); + }); +}); From 3142a3f19433b2ff0689dbf09d2bf5ce95b5a22e Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Sat, 5 Aug 2017 12:14:18 -0700 Subject: [PATCH 2/5] cleanup / fix tests --- test/tests.transform.js | 135 ++++++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 45 deletions(-) diff --git a/test/tests.transform.js b/test/tests.transform.js index cbaa8baf0..c2d94a444 100644 --- a/test/tests.transform.js +++ b/test/tests.transform.js @@ -48,56 +48,101 @@ describe("_blockHoist nodes", function() { }); }); -describe("marked", function() { - it("should work with a single function", function() { - var ast = recast.parse('function* foo(){};', { - parser: require("babylon") +context("functions", function() { + var itMarksCorrectly = function(marked, varName) { + // marked should be a VariableDeclarator + n.VariableDeclarator.assert(marked); + + // using our variable name + assert.strictEqual(marked.id.name, varName); + + // assiging a call expression to regeneratorRuntime.mark() + n.CallExpression.assert(marked.init); + assert.strictEqual(marked.init.callee.object.name, 'regeneratorRuntime') + assert.strictEqual(marked.init.callee.property.name, 'mark') + + // with said call expression marked as a pure function + assert.strictEqual(marked.init.leadingComments[0].value, '#__PURE__'); + }; + + describe("function declarations", function() { + it("should work with a single function", function() { + var ast = recast.parse('function* foo(){};', { + parser: require("babylon") + }); + + // get our declarations + const declaration = transform(ast).program.body[0]; + n.VariableDeclaration.assert(declaration); + const declarations = declaration.declarations; + + // verify our declaration is marked correctly + itMarksCorrectly(declarations[0], '_marked'); + + // and has our function name as its first argument + assert.strictEqual(declarations[0].init.arguments[0].name, 'foo'); }); - const code = recast.print(transform(ast)).code; - assert.notStrictEqual( - code.indexOf( - 'var _marked = regeneratorRuntime.mark(foo);' - ), -1 - ); - }); - it("should work with multiple functions", function() { - var ast = recast.parse([ - 'function* foo(){};', - 'function* bar() {};' - ].join("\n"), { - parser: require("babylon") + + it("should work with multiple functions", function() { + var ast = recast.parse([ + 'function* foo(){};', + 'function* bar() {};' + ].join("\n"), { + parser: require("babylon") + }); + + // get our declarations + const declaration = transform(ast).program.body[0]; + n.VariableDeclaration.assert(declaration); + const declarations = declaration.declarations; + + // verify our declarations are marked correctly and have our function name + // as their first argument + itMarksCorrectly(declarations[0], '_marked'); + n.Identifier.assert(declarations[0].init.arguments[0]); + assert.strictEqual(declarations[0].init.arguments[0].name, 'foo'); + + itMarksCorrectly(declarations[1], '_marked2'); + n.Identifier.assert(declarations[1].init.arguments[0]); + assert.strictEqual(declarations[1].init.arguments[0].name, 'bar'); }); - const code = recast.print(transform(ast)).code; - assert.notStrictEqual( - code.indexOf( - 'var _marked = regeneratorRuntime.mark(foo), _marked2 = regeneratorRuntime.mark(bar);' - ), -1 - ); }); -}); -// TODO - how do i get recast to return comments? -describe("pure", function() { - xit("should work with a function expression", function() { - var ast = recast.parse('var a = function* foo(){};', { - parser: require("babylon") + describe("function expressions", function() { + it("should work with a named function", function() { + var ast = recast.parse('var a = function* foo(){};', { + parser: require("babylon") + }); + + // get our declarations + const declaration = transform(ast).program.body[0]; + n.VariableDeclaration.assert(declaration); + const declarator = declaration.declarations[0]; + + // verify our declaration is marked correctly + itMarksCorrectly(declarator, 'a'); + + // and that our first argument is our original function expression + n.FunctionExpression.assert(declarator.init.arguments[0]); + assert.strictEqual(declarator.init.arguments[0].id.name, 'foo'); }); - const code = recast.print(transform(ast)).code; - assert.notStrictEqual( - code.indexOf( - 'var a = /*#__PURE__*/regeneratorRuntime.mark(function foo() {' - ), -1 - ); - }); - xit("should work with a function declaration", function() { - var ast = recast.parse('function* foo(){};', { - parser: require("babylon") + + it("should work with an anonymous function", function() { + var ast = recast.parse('var a = function* (){};', { + parser: require("babylon") + }); + + // get our declarations + const declaration = transform(ast).program.body[0]; + n.VariableDeclaration.assert(declaration); + const declarator = declaration.declarations[0]; + + // verify our declaration is marked correctly + itMarksCorrectly(declarator, 'a'); + + // and that our first argument is our original function expression + n.FunctionExpression.assert(declarator.init.arguments[0]); + assert.strictEqual(declarator.init.arguments[0].id.name, '_callee'); }); - const code = recast.print(transform(ast)).code; - assert.notStrictEqual( - code.indexOf( - 'var a = /*#__PURE__*/Object.defineProperty( /*#__PURE__*/regeneratorRuntime.mark' - ), -1 - ); }); }); From 215c1ef4f83ee1e26c5cffdf61fcb1386ee2ed65 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Thu, 10 Aug 2017 10:27:35 -0700 Subject: [PATCH 3/5] add actual dead code removal test with uglify --- package.json | 3 +- test/tests.transform.js | 87 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 0e6239ffe..16ab35fab 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,8 @@ "regenerator-preset": "^0.9.6", "regenerator-runtime": "^0.10.5", "regenerator-transform": "^0.9.12", - "through": "^2.3.8" + "through": "^2.3.8", + "uglify-js": "^3.0.27" }, "devDependencies": { "babel-cli": "^6.9.0", diff --git a/test/tests.transform.js b/test/tests.transform.js index c2d94a444..849874252 100644 --- a/test/tests.transform.js +++ b/test/tests.transform.js @@ -3,6 +3,9 @@ var recast = require("recast"); var types = recast.types; var n = types.namedTypes; var transform = require("..").transform; +var compile = require("..").compile; + +var UglifyJS = require("uglify-js"); describe("_blockHoist nodes", function() { it("should be hoisted to the outer body", function() { @@ -48,6 +51,88 @@ describe("_blockHoist nodes", function() { }); }); +describe("uglifyjs dead code removal", function() { + function uglifyAndParse(file1, file2) { + var code = { + "file1.js": file1, + "file2.js": file2 + }; + + var options = { + toplevel: true, + // don't mangle function or variable names so we can find them + mangle: false, + output: { + // make it easier to parse the output + beautify: true + } + }; + + // uglify our code + var result = UglifyJS.minify(code, options); + + // parse and return the output + return recast.parse(result.code, { + parser: require("babylon") + }); + } + + it("works with function expressions", function() { + var file1 = compile([ + 'var foo = function* () {};', + 'var bar = function* () {};' + ].join("\n")).code; + var file2 = compile('console.log(foo());').code; + + var ast = uglifyAndParse(file1, file2); + + // the results should have a single variable declaration + var variableDeclarations = ast.program.body.filter(function(b) { + return b.type === 'VariableDeclaration'; + }); + assert.strictEqual(variableDeclarations.length, 1); + assert.strictEqual(variableDeclarations[0].declarations.length, 1); + var declaration = variableDeclarations[0].declarations[0]; + + // named foo + assert.strictEqual(declaration.id.name, 'foo'); + }); + + it("works with function definitions", function() { + var file1 = compile([ + 'function* foo() {};', + 'function* bar() {};' + ].join("\n")).code; + + var file2 = compile('console.log(foo());').code; + + var ast = uglifyAndParse(file1, file2); + + // the results should have our foo() function + assert.ok(ast.program.body.some(function(b) { + return b.type === 'FunctionDeclaration' && b.id.name === 'foo'; + })); + + // but not our bar() function + assert.ok(!ast.program.body.some(function(b) { + return b.type === 'FunctionDeclaration' && b.id.name === 'bar'; + })); + + // and a single mark declaration + var variableDeclarations = ast.program.body.filter(function(b) { + return b.type === 'VariableDeclaration'; + }); + assert.strictEqual(variableDeclarations.length, 1); + var declarations = variableDeclarations[0].declarations; + assert.strictEqual(declarations.length, 1); + var declaration = declarations[0]; + + // with our function name as an argument' + assert.strictEqual(declaration.init.arguments.length, 1); + assert.strictEqual(declaration.init.arguments[0].name, 'foo'); + }); +}) + context("functions", function() { var itMarksCorrectly = function(marked, varName) { // marked should be a VariableDeclarator @@ -85,7 +170,7 @@ context("functions", function() { it("should work with multiple functions", function() { var ast = recast.parse([ - 'function* foo(){};', + 'function* foo() {};', 'function* bar() {};' ].join("\n"), { parser: require("babylon") From d1ce442ce5feaa3111f3706f4370937d13a3bfe5 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Aug 2017 13:11:04 -0400 Subject: [PATCH 4/5] Move uglify-js to devDependencies and other minor tweaks. --- package.json | 6 +++--- test/tests.transform.js | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 16ab35fab..70b41d85a 100644 --- a/package.json +++ b/package.json @@ -44,8 +44,7 @@ "regenerator-preset": "^0.9.6", "regenerator-runtime": "^0.10.5", "regenerator-transform": "^0.9.12", - "through": "^2.3.8", - "uglify-js": "^3.0.27" + "through": "^2.3.8" }, "devDependencies": { "babel-cli": "^6.9.0", @@ -57,7 +56,8 @@ "lerna": "^2.0.0", "mocha": "^2.3.4", "promise": "^7.0.4", - "semver": "^5.0.3" + "semver": "^5.0.3", + "uglify-js": "^3.0.27" }, "license": "BSD", "engines": { diff --git a/test/tests.transform.js b/test/tests.transform.js index 849874252..a66b5c7be 100644 --- a/test/tests.transform.js +++ b/test/tests.transform.js @@ -76,7 +76,7 @@ describe("uglifyjs dead code removal", function() { parser: require("babylon") }); } - + it("works with function expressions", function() { var file1 = compile([ 'var foo = function* () {};', @@ -98,7 +98,7 @@ describe("uglifyjs dead code removal", function() { assert.strictEqual(declaration.id.name, 'foo'); }); - it("works with function definitions", function() { + it("works with function declarations", function() { var file1 = compile([ 'function* foo() {};', 'function* bar() {};' @@ -134,7 +134,7 @@ describe("uglifyjs dead code removal", function() { }) context("functions", function() { - var itMarksCorrectly = function(marked, varName) { + function marksCorrectly(marked, varName) { // marked should be a VariableDeclarator n.VariableDeclarator.assert(marked); @@ -148,12 +148,12 @@ context("functions", function() { // with said call expression marked as a pure function assert.strictEqual(marked.init.leadingComments[0].value, '#__PURE__'); - }; + } describe("function declarations", function() { it("should work with a single function", function() { var ast = recast.parse('function* foo(){};', { - parser: require("babylon") + parser: require("babylon") }); // get our declarations @@ -162,7 +162,7 @@ context("functions", function() { const declarations = declaration.declarations; // verify our declaration is marked correctly - itMarksCorrectly(declarations[0], '_marked'); + marksCorrectly(declarations[0], '_marked'); // and has our function name as its first argument assert.strictEqual(declarations[0].init.arguments[0].name, 'foo'); @@ -173,7 +173,7 @@ context("functions", function() { 'function* foo() {};', 'function* bar() {};' ].join("\n"), { - parser: require("babylon") + parser: require("babylon") }); // get our declarations @@ -183,11 +183,11 @@ context("functions", function() { // verify our declarations are marked correctly and have our function name // as their first argument - itMarksCorrectly(declarations[0], '_marked'); + marksCorrectly(declarations[0], '_marked'); n.Identifier.assert(declarations[0].init.arguments[0]); assert.strictEqual(declarations[0].init.arguments[0].name, 'foo'); - itMarksCorrectly(declarations[1], '_marked2'); + marksCorrectly(declarations[1], '_marked2'); n.Identifier.assert(declarations[1].init.arguments[0]); assert.strictEqual(declarations[1].init.arguments[0].name, 'bar'); }); @@ -196,7 +196,7 @@ context("functions", function() { describe("function expressions", function() { it("should work with a named function", function() { var ast = recast.parse('var a = function* foo(){};', { - parser: require("babylon") + parser: require("babylon") }); // get our declarations @@ -205,7 +205,7 @@ context("functions", function() { const declarator = declaration.declarations[0]; // verify our declaration is marked correctly - itMarksCorrectly(declarator, 'a'); + marksCorrectly(declarator, 'a'); // and that our first argument is our original function expression n.FunctionExpression.assert(declarator.init.arguments[0]); @@ -214,7 +214,7 @@ context("functions", function() { it("should work with an anonymous function", function() { var ast = recast.parse('var a = function* (){};', { - parser: require("babylon") + parser: require("babylon") }); // get our declarations @@ -223,7 +223,7 @@ context("functions", function() { const declarator = declaration.declarations[0]; // verify our declaration is marked correctly - itMarksCorrectly(declarator, 'a'); + marksCorrectly(declarator, 'a'); // and that our first argument is our original function expression n.FunctionExpression.assert(declarator.init.arguments[0]); From 48485316e89ac7a25eeb98c83591a73594b88230 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Tue, 15 Aug 2017 13:55:34 -0400 Subject: [PATCH 5/5] Move more logic into getMarkedFunctionId helper function. --- packages/regenerator-transform/src/visit.js | 85 ++++++++++----------- 1 file changed, 41 insertions(+), 44 deletions(-) diff --git a/packages/regenerator-transform/src/visit.js b/packages/regenerator-transform/src/visit.js index ef7d27139..f69d7d67b 100644 --- a/packages/regenerator-transform/src/visit.js +++ b/packages/regenerator-transform/src/visit.js @@ -8,6 +8,8 @@ * the same directory. */ +"use strict"; + import assert from "assert"; import * as t from "babel-types"; import { hoist } from "./hoist"; @@ -15,8 +17,6 @@ import { Emitter } from "./emit"; import replaceShorthandObjectMethod from "./replaceShorthandObjectMethod"; import * as util from "./util"; -let getMarkInfo = require("private").makeAccessor(); - exports.name = "regenerator-transform"; exports.visitor = { @@ -180,61 +180,58 @@ function getOuterFnExpr(funPath) { if (node.generator && // Non-generator functions don't need to be marked. t.isFunctionDeclaration(node)) { - let pp = funPath.findParent(function (path) { - return path.isProgram() || path.isBlockStatement(); - }); - - if (!pp) { - return node.id; - } + // Return the identifier returned by runtime.mark(). + return getMarkedFunctionId(funPath); + } - let markDecl = getRuntimeMarkDecl(pp); - let declarations = markDecl.declarations; - let index = declarations.length; + return node.id; +} - // this can change from pass to pass if something else is altering our body - // so we have to look it up every time :/ - let declarationsIndex = pp.node.body.findIndex((p) => { - return t.isVariableDeclaration(p) && p.declarations === declarations; - }); - assert(!isNaN(declarationsIndex)); +const getMarkInfo = require("private").makeAccessor(); - // get a new unique id for our marked variable - let markedId = pp.scope.generateUidIdentifier("marked"); +function getMarkedFunctionId(funPath) { + const node = funPath.node; + t.assertIdentifier(node.id); - // now push our new declaration into our list - declarations.push(t.variableDeclarator( - markedId, - t.callExpression(util.runtimeProperty("mark"), [node.id]) - )); + const blockPath = funPath.findParent(function (path) { + return path.isProgram() || path.isBlockStatement(); + }); - // assemble the path to our mark declarations - let bodyPathName = 'body.' + String(declarationsIndex) + '.declarations.' + String(index) + '.init'; - let bodyPath = pp.get(bodyPathName); + if (!blockPath) { + return node.id; + } - // mark our call to util.runtimeProperty("mark") as pure to enable tree-shaking - bodyPath.addComment("leading", "#__PURE__"); + const block = blockPath.node; + assert.ok(Array.isArray(block.body)); - // and return our new id - return markedId; + const info = getMarkInfo(block); + if (!info.decl) { + info.decl = t.variableDeclaration("var", []); + blockPath.unshiftContainer("body", info.decl); + info.declPath = blockPath.get("body.0"); } - return node.id; -} + assert.strictEqual(info.declPath.node, info.decl); -function getRuntimeMarkDecl(blockPath) { - let block = blockPath.node; - assert.ok(Array.isArray(block.body)); + // Get a new unique identifier for our marked variable. + const markedId = blockPath.scope.generateUidIdentifier("marked"); + const markCallExp = t.callExpression( + util.runtimeProperty("mark"), + [node.id] + ); - let info = getMarkInfo(block); - if (info.decl) { - return info.decl; - } + const index = info.decl.declarations.push( + t.variableDeclarator(markedId, markCallExp) + ) - 1; + + const markCallExpPath = + info.declPath.get("declarations." + index + ".init"); + + assert.strictEqual(markCallExpPath.node, markCallExp); - info.decl = t.variableDeclaration("var", []) - blockPath.unshiftContainer("body", info.decl); + markCallExpPath.addComment("leading", "#__PURE__"); - return info.decl; + return markedId; } function renameArguments(funcPath, argsId) {