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.

Sign up for GitHub

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

Jump to bottom

Fix remove console.* statements #421

Merged
merged 6 commits into from Feb 20, 2017
Merged

Fix remove console.* statements #421

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a1c5eb9
Fix remove console.* statements
boopathi Feb 12, 2017
add99ad
ES5ify arrows
boopathi Feb 13, 2017
127be4c
Add call and apply
boopathi Feb 18, 2017
ad95eef
Remove early exit that doesn't make much sense
boopathi Feb 19, 2017
7bb7462
Handle local declarations of console
boopathi Feb 19, 2017
3937db7
Handle assignment expressions
boopathi Feb 20, 2017
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
49 changes: 47 additions & 2 deletions packages/babel-plugin-transform-remove-console/__tests__/remove-console-test.js
View file
Open in desktop
Expand Up @@ -37,7 +37,7 @@ describe("remove-console-plugin", () => {

const expected = unpad(`
function foo() {
true;
true && void 0;
blah();
}
`);
Expand All @@ -51,7 +51,7 @@ describe("remove-console-plugin", () => {
`);

const expected = unpad(`
true;
true && void 0;
blah();
`);
expect(transform(source)).toBe(expected);
Expand Down Expand Up @@ -89,4 +89,49 @@ describe("remove-console-plugin", () => {
`);
expect(transform(source).trim()).toBe(expected);
});

it("should remove console.* assignments to other variables", () => {
const source = unpad(`
const a = console.log;
a();
const b = console.log.bind(console);
b("asdf");
var x = console.log ? console.log('log') : foo();
function foo() {
if (console.error) {
console.error("Errored");
}
}
console.log.call(console, "foo");
console.log.apply(null, {});
`);
const expected = unpad(`
const a = function () {};
a();
const b = function () {};
b("asdf");
var x = function () {} ? void 0 : foo();
function foo() {
if (function () {}) {}
}
`);
expect(transform(source)).toBe(expected);
});

it("should NOT remove local bindings of name console", () => {
const source = unpad(`
function foo(console) {
console.foo("hi");
const bar = console.foo.bind(console);
}
function bar(a) {
const { console } = a;
a.b = console => console.bar("bar");
if (console.foo.call(console, "bar")) {
return;
}
}
`);
expect(transform(source)).toBe(source);
});
});
59 changes: 56 additions & 3 deletions packages/babel-plugin-transform-remove-console/src/index.js
View file
Open in desktop
@@ -1,14 +1,67 @@
"use strict";

module.exports = function() {
module.exports = function({ types: t }) {
return {
name: "transform-remove-console",
visitor: {
CallExpression(path) {
if (path.get("callee").matchesPattern("console", true)) {
path.remove();
const callee = path.get("callee");

if (!callee.isMemberExpression()) return;

if (isConsole(callee)) {
// console.log()
if (path.parentPath.isExpressionStatement()) {
path.remove();
} else {
path.replaceWith(createVoid0());
}
} else if (isConsoleBind(callee)) {
// console.log.bind()
path.replaceWith(createNoop());
}
},
MemberExpression: {
exit(path) {
if (isConsole(path) && !path.parentPath.isMemberExpression()) {
path.replaceWith(createNoop());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks not work with console.err = xx (React Native have similar usage), maybe we can just parentPath.remove() if path key is left and parentPath is assignment expression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, So, I would assume it to be replaced like this -

// in
console.foo = foo;
// out
console.foo = function () {};

which looks like the right thing to do. because it is hard to detect all usages of console.foo in code

const x = "foo";
const y = console;
y[x]();

and will lead to runtime error.

}
}
},
};

function isGlobalConsoleId(id) {
const name = "console";
return id.isIdentifier({ name })
&& !id.scope.getBinding(name)
&& id.scope.hasGlobal(name);
}

function isConsole(memberExpr) {
Copy link

Choose a reason for hiding this comment

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

Seems this needs to check for console.log.call and console.log.apply.

Copy link

Choose a reason for hiding this comment

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

And the test needs to check both a = console.log.call and console.log.call()

const object = memberExpr.get("object");
if (isGlobalConsoleId(object)) return true;

const property = memberExpr.get("property");
return isGlobalConsoleId(object.get("object"))
&& (
property.isIdentifier({ name: "call" })
|| property.isIdentifier({ name: "apply" })
);
}

function isConsoleBind(memberExpr) {
const object = memberExpr.get("object");
return object.isMemberExpression()
&& isGlobalConsoleId(object.get("object"))
&& memberExpr.get("property").isIdentifier({ name: "bind" });
}

function createNoop() {
return t.functionExpression(null, [], t.blockStatement([]));
}

function createVoid0() {
return t.unaryExpression("void", t.numericLiteral(0));
}
};