Add semi-colon to fix webpack usage

[benhartley]

Fix for this issue.

Webpack currently throws the following error when trying to parse this module:

ERROR in ./~/newrelic/index.js
Module parse failed: newrelic/index.js Line 18: Illegal return statement 
You may need an appropriate loader to handle this file type. 
| ) 
| module.exports = require.cache.__NR_cache 
| return 
| } 

[wraithan]

I'm unclear how this is a bug in our agent. That is valid JS that cannot be parsed ambiguously by a valid parser to the best of my knowledge. Do you know what ASI gotcha this is hitting?

I hesitate to account for parser errors in other JS implementations without solid reasoning. Can I ask why you are using webpack with node? AFAIK it is a very uncommon path.

[wraithan]

Ah, a friend pointed out why this could be problem. It is a return outside of a function. But because node wraps all modules in functions this is ok in node.

How the semicolon fixes it, I have no idea though, seems very black magic-y. It is possible for us to account for this, but I'm unlikely to do so by adding a magic semicolon. A better fix would be to wrap up that init stuff in a function and call it.

[benhartley]

It would be good to see a fix for it regardless of how you choose to implement it so that we can switch back to using your version instead of my fork.

As for server-side webpack - once uncommon perhaps, but increasingly less so - I'd expect to see more of it over time as people want to transpile different things to javascript.

[wraithan]

So far this is our second request for node agent compatibility with webpack/browserify that I'm aware of. We've see many times when people thought they wanted that but what they wanted was our browser agent. I'll add this as a task for one of our upcoming bug sprints but currently we are heads down on schedule roadmap items.

Webpack is not a supported path currently, nor any other compiler that isn't node itself. I have my doubts that it will work properly with all webpack use cases because of the myriad of things webpack supports. If it proxies require or other core APIs, renames files, etc, we can't guarantee our instrumentation will work or that it wont crash for other reasons.

We honestly do our best to accommodate the widest breadth of users as possible (hence the node 0.8 -> io.js 3.x version support and wide variety of datastore drivers). But it is very hard to predict what the various compilers that target JS will do with code and unfortunately the kind of work we do is very finicky, otherwise I'd have us using babel so we can use those delicious ES6 features like fat arrows and destructuring.

[wraithan]

While a semicolon fix for webpack suggests that their parser has a bug (ASI is likely landing in the wrong place due to unexpected use of return) I've bumped up the priority on our internal issue to correct the real problem and find any related potential problems.

JS parsers that are not node aware, wont know you are in in a function while they parse your JS. This means return keyword should not be there. Related break and continue are also disallowed outside of their respective control flow. I've elected this ticket for our next bug sprint and we'll make sure there aren't any related cases as we fix this.

I unfortunately can't give a timeline for release of a fix, because the exact date of our next bug sprint is not scheduled yet as well as I may be overridden on priority by business needs.

I'm going to close this PR. If you decide to take a stab at fixing the top level return problem, please open a new PR and we'll take a look at it. Fundamentally, I want to fix the underlying problem rather than relying on a quirk of the current webpack js parser.