Fix for #7: Not as optimized for Webpack as it could be.

[aickin]

Adds ability to wrap functions that are elements of an array literal in an argument list.

I refactored some of the if-else logic into named functions that were easier for me to understand, but I'm not sure if they will be easier for others or not. Feedback welcome.

npm run test runs green and npm run coverage gives 100%.

Running on the benchmark repo from The Cost of Small Modules on my MBP (and snipping out non-webpack versions for space), I get:

Chrome 54.0.2840.71

100 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,2.51,1.55,4.06
webpack-optimized,2.88,0.26,3.14
1000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,7.33,16.92,24.25
webpack-optimized,9.4,1.65,11.05
5000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,35.44,47.64,83.09
webpack-optimized,44.72,6.25,50.97

Firefox 48.0.2

100 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,2.07,0.93,3
webpack-optimized,2.29,0.45,2.74
1000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,5.28,6.18,11.45
webpack-optimized,8.5,1.63,10.13
5000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,22.03,29.4,51.43
webpack-optimized,36.4,7.36,43.76

Safari 9.1.3

100 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,2.19,0.39,2.58
webpack-optimized,2.16,0.38,2.54
1000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,3.34,1.52,4.86
webpack-optimized,2.69,1.69,4.38
5000 modules,,,
Bundler,Load time (ms),Run time (ms),Total time (ms)
webpack,6.97,9.71,16.67
webpack-optimized,6.19,8.26,14.45

Thanks for all your work!

[nolanlawson]

This is awesome!! 🎉 I'm eager to merge, but have a few suggestions:

1. We should add a new benchmark file built by Webpack (React DOM maybe? the "cost of modules" file is too artificial IMO) and re-run the benchmarks in the README. I can do that this weekend.
2. I'd like to see more test cases to confirm that functions within arrays that are not arguments to a function do not result in unnecessary parens. Probably a super edge case, but I'd really like to optimize just for this particular webpack case and not add parens where they're not required.

All in all though, great work and if you don't have time to address # 2 then I can take care of it after I merge.

[aickin]

1. I'm fairly certain that react-dom/dist/react-dom.js is just a very small non-bundled file that just pulls a property out of react.js, and react/dist/react.js is packaged by browserify. I don't know of any libraries offhand that use webpack. To test webpack, I've been using ColorFlood, which is a simple create-react-app application, but I'm sure there are lots of other good examples.
2. This is good idea; I'm happy to add a couple test cases in the next few days.

[aickin]

I added four negative test cases for array literals. I feel like I didn't have a ton of creativity in making these test cases, though, so let me know if there are some more ones you'd like!

[aickin]

Actually, it just occurred to me that this could be designed to be more specific to webpack if we want. Right now the logic of this PR is:

• Wrap any function that is an element of an array literal which is a function parameter.

But if we wanted to make it even more specific to webpack, we could make the logic instead be:

• Wrap any function that is an element of an array literal which is a function parameter to an IIFE.

I'm inclined to change it that way. What do you think?

[aickin]

I coded up a more webpack-specific version in PR #33, which is now my preference, I think.

[aickin]

Aaaaaand I went ahead and added a webpack benchmark in PR #34 in case you want to use it. (I unexpectedly had a bunch of time this weekend and have found working on this surprisingly fun...)

[nolanlawson]

Yeah I agree that #33 is a more targeted solution. I'd prefer for this tool to be conservative to avoid over-parsing functions. Thanks for offering options with multiple PRs! :)