grunt-remove-logging icon indicating copy to clipboard operation
grunt-remove-logging copied to clipboard

removal is overly greedy when handling closing braces

Open timotheeg opened this issue 12 years ago • 5 comments

Adding the following test breaks and result in a syntax error in the resulting processed javascript:

[
  'function foo(msg){console.log(msg)}\nfunction bar(){}',
  {},
  'function foo(msg){}\nfunction bar(){}',
],

incorrectly generates:

function foo(msg){{}

timotheeg avatar Feb 14 '13 09:02 timotheeg

real world file that gets broken because of said issue: https://github.com/cloudhead/less.js/blob/master/lib/less/browser.js

I'm not saying the js in that file is good btw (adding a semi-colon after the console.log statement fixes the issue), but projects may import less.js and have their grunt build break because of the issue mentioned.

timotheeg avatar Feb 14 '13 09:02 timotheeg

This issue especially arises when using the removelogging with uglified files. Here the trailing semicolon gets removed when there is no need for them.

For example:

fail(function(a,b,c){console.log("Error"),console.log(a),console.log(b),console.log(c)})});foo(bar);

The expected result would be:

fail(function(a,b,c){(0);})});foo(bar);

but is instead:

fail(function(a,b,c){foo(bar);

This annoyingly prevents us from including your removelogging in our grunt workflow. A fix would be much appreciated. And thanks anyway for your work.

SerafinDinges avatar Dec 16 '13 09:12 SerafinDinges

Yeah, one limitation of this plugin is that it requires a trailing semicolon since the regex is non-greedy. Until I can figure out how to fix that (pulls welcome) I suggest running this task before minification.

ehynds avatar Dec 17 '13 15:12 ehynds

The heart of the issue, imho, is that the removing log statements cannot be done reliably with regexps alone. But I do realize that adding a full js tokenizer in there is a big job :(

timotheeg avatar Dec 18 '13 01:12 timotheeg

I too have been having issues doing a removelogging after uglify. The current solution I'm using is to just put removelogging before the uglify task in the build order. This feels fragile though.

Soviut avatar Mar 08 '14 19:03 Soviut