Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant use strict (Fix #542) #553

Merged
merged 1 commit into from May 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -2686,4 +2686,28 @@ describe("dce-plugin", () => {
`);
expect(transform(source)).toBe(expected);
});

it("should remove unnecessary use strict directives", () => {
const source = unpad(`
function foo() {
"use strict";
function bar() {
"use strict";
bar();
}
bar.call();
}
`);
const expected = unpad(`
function foo() {
"use strict";

function bar() {
bar();
}
bar.call();
}
`);
expect(transform(source)).toBe(expected);
});
});
13 changes: 13 additions & 0 deletions packages/babel-plugin-minify-dead-code-elimination/src/index.js
Expand Up @@ -2,6 +2,7 @@

const some = require("lodash.some");
const { markEvalScopes, hasEval } = require("babel-helper-mark-eval-scopes");
const removeUseStrict = require("./remove-use-strict");

function prevSiblings(path) {
const parentPath = path.parentPath;
Expand Down Expand Up @@ -743,6 +744,18 @@ module.exports = ({ types: t, traverse }) => {
return {
name: "minify-dead-code-elimination",
visitor: {
Function: {
exit(path) {
/**
* Use exit handler to traverse in a dfs post-order fashion
* to remove use strict
*/
const body = path.get("body");
if (body.isBlockStatement()) {
removeUseStrict(body);
}
}
},
IfStatement: {
exit(path) {
const consequent = path.get("consequent");
Expand Down
@@ -0,0 +1,56 @@
"use strict";

module.exports = removeUseStrict;

const newIssueUrl = "https://github.com/babel/babili/issues/new";

const useStrict = "use strict";

/**
* Remove redundant use strict
* If the parent has a "use strict" directive, it is not required in
* the children
*
* @param {NodePath} block BlockStatement
*/
function removeUseStrict(block) {
if (!block.isBlockStatement()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this check even needed if its already checked before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is a separate file. to not shoot ourselves in foot by calling removeUseStrict(SomeOtherPath)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! didn't realise it was a separate module.

throw new Error(
`Received ${block.type}. Expected BlockStatement. ` +
`Please report at ${newIssueUrl}`
);
}

const useStricts = getUseStrictDirectives(block);

// early exit
if (useStricts.length < 1) return;

// only keep the first use strict
if (useStricts.length > 1) {
for (let i = 1; i < useStricts.length; i++) {
useStricts[i].remove();
}
}

// check if parent has an use strict
if (hasStrictParent(block)) {
useStricts[0].remove();
}
}

function hasStrictParent(path) {
return path.findParent(
parent => parent.isBlockStatement() && isStrict(parent)
);
}

function isStrict(block) {
return getUseStrictDirectives(block).length > 0;
}

function getUseStrictDirectives(block) {
return block.get("directives").filter(directive => {
return directive.node.value.value === useStrict;
});
}