[renderjson.js] Changed export to match umd

[dozoisch]

I was having issue with requiring renderjson.js with webpack/babel when minifying because it was changing the way the export worked. I changed the way the module is define to follow UMD repository. https://github.com/umdjs/umd/blob/a858c346be351a26a86a791cf9fb2dd9fb93d7f1/templates/returnExportsGlobal.js

[dozoisch]

@caldwell Anything I can do to get this merged?

[caldwell]

Oops, I shouldn't have closed this. I added amd support but that isn't what the PR was about.

The reason I didn't just immediately merge was because I don't want to add so much boilerplate just for module support. But I also don't understand the crux of the problem you were running into—I have a (non-committed) webpack testcase but it works for me, so I'm curious what errors you are seeing.

I'm sorry that I've appeared unresponsive.

[dozoisch]

@caldwell

When I was importing for webpack the export line became this:

({}).exports = {}.renderjson = function () {

Where before it was referencing module correctly. (See https://github.com/mongo-express/mongo-express/issues/330#issuecomment-294280905)

I can try again with the new changes you made, but the idea behind my commit was to follow the UMD repository conventions.

[caldwell]

Hmm, interesting. The problem seems to be something (the minifier?) doing an unsafe optimization and assuming that modules is undefined.

Maybe it's getting minified before it get's webpacked? At that point the variable stuff would be global and it's doing an optimization that's valid in global context but isn't valid in the final function context. Even so, it's a shaky optimization because the global variable could be set by some other script.

I grabbed the mongo-express repo and pointed it to renderjson 1.3.1 and the rearranging of the code seems to cause it to not optimize t out. The new renderjson code (built with npm run build):

if (define) define({renderjson:renderjson})
else (module||{}).exports = (window||{}).renderjson = renderjson;

now gets minimized down to:

n?n({renderjson:o}):(t||{}).exports={}.renderjson=o

Is this fixed? Or was there something else that needed to happen to tickle the bug?

[dozoisch]

Well, I assume the optimizer used the fact that module was define the line before (var module;)

https://github.com/caldwell/renderjson/blob/master/renderjson.js#L59

and considered module to be a local variable. I'll update my node modules and try again to see if this was fixed at some point in the minifier and I was just using an old version.

[caldwell]

The problem is that var module in the global scope doesn't override module in the global scope—try it in the browser console:

var a=3;
window.a;

It'll show 3. Same with function parameters: function f(x) { var x; return x } will work just fine.

That's why I'm blaming the minifier.

But then again I can't get it fail locally:

Interestingly uglify doesn't do this when it's just renderjson by itself. uglifyjs renderjson.js --compress --mangle produces:

var module,window;(module||{}).exports=(window||{}).function(){

And if I wrap renderjson in a webpack-like function by adding:

function x(module, exports) {

I get:

function x(t,n){var t,r;(t||{}).exports=(r||{}).renderjson=function(){

In both cases, even the window parameter isn't getting optimized away. So I'm not sure what's doing that in the full webpack stack (though I clearly see it when I grab the 0.39.1 version of mongo-express from npm).

[dozoisch]

Thanks for your time! I'll try it again tonight with up to date modules to see what happens.

[caldwell]

I'm going to close this since I haven't heard anything since May. Feel free to re-open.