YahooArchive / strip-loader

Webpack loader to strip arbitrary functions out of your production code.
Other
282 stars 14 forks source link

Use recast to strip functions #7

Open Vijar opened 9 years ago

Vijar commented 9 years ago

I got this working with recast. Mainly took inspiration from fixmyjs.

This would fix #4

However, this is now backwards incompatible since we can no longer strip console.log or any other CallExpressions that are considered MemberExpressions. We can only strip Identifiers such as 'debug' or 'assert'.

I actually don't think this loader should be stripping console.log statements anyway, so I actually don't mind the breaking version.

@gpbl, if you can test this branch for your code and give me a +1, I will merge.

yahoocla commented 9 years ago

CLA is valid!

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-8.33%) to 91.67% when pulling 98d8b2dc3ae234cc15bafc877774dabdcd8b6a32 on recast into 5020e4242aa3933d9cb7e893fed4542459b00c6c on master.

gpbl commented 9 years ago

Thanks @Vijar trying it now: I have an issue with recast and babeljs, let me investigate first :-) I do agree console.log shouldn't be stripped by this loader, it's also a job for the Uglifier.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-4.11%) to 95.89% when pulling 5b932da594fd5cefc82d520b8a8d59bbe308cff8 on recast into 5020e4242aa3933d9cb7e893fed4542459b00c6c on master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-4.11%) to 95.89% when pulling 5b932da594fd5cefc82d520b8a8d59bbe308cff8 on recast into 5020e4242aa3933d9cb7e893fed4542459b00c6c on master.

gpbl commented 9 years ago

@Vijar I believe recast introduces a new level of complexities :-)

This code with an inline jsx comment didn't work with the strip-loader when running the babeljs-loader first:

render() {
  return (
    <p>
     { /* inline jsx comment */ }
    </p>
  );
}

recast throws an error:

Module build failed: Error: Comment location overlaps with node location

because babel puts the comment on the bottom of the file (https://github.com/babel/babel/issues/459, https://github.com/babel/babel/issues/672). So recast is safer than a regexp, on the other side it put some limitations we should be aware of.

Vijar commented 9 years ago

@gpbl, I actually did try to preserve comments but recast doesn't play nice with comments at all! :(

I think we may have to do what fixmyjs guys did, create a v2.0 release with recast because it is safer, but still maintain a legacy version which still uses your wider regexp PR. We can address the pros/cons of each in the README.

gpbl commented 9 years ago

@Vijar :+1: thanks!