Test require("dustjs-helpers") clobbers global dust object in node

[kate2753]

Updated test spec runner for node to test that require("dustjs-helpers") adds helpers into global.dust object. #72 has the context for this change

[prashn64]

lgtm

[jimmyhchan]

see comment in #72 👎

[kate2753]

I think this change still applies. I'm not changing what this code does, just making what is going on here more visible.

Currently spec runner for node relies on dust being a global variable. Global dust variable is used in renderTestSpec.js and helpersTests.js.

If we wanted to change this it, we would update old and new test runners for node test\server.js and test\jasmine-test\server\specRunner.js not to have dust as a global. We would then have to explicitly declare dust variable in renderTestSpec.js and helpersTests.js . Something like

var dust = typeof exports !== 'undefined' ? require('dustjs-linkedin') : dust;

Is this approach better than what we have? To me it seems more complicated and harder to follow.

[jimmyhchan]

I see. got confused with the solutions from #72. sorry for the 👎 . This makes sense.

[jimmyhchan]

Do we use this for browser testing as well? we'll run into an issue with window vs global

[kate2753]

Yup, we use both renderTestSpec.js and helpersTests.js for browser testing. I guess we could wrap both of the files in UMD style wrapper.

[kate2753]

Updated the diff.
Moved module.exports=dust;, cleaned up node spec runners and added unit tests to test #72. Let me know if changes look good, otherwise I can go back to the code in original commit for this.

[jimmyhchan]

should we check module or exports here?

[kate2753]

We should. Updated.

[jimmyhchan]

looks good otherwise.

[framp]

Is there any chance of this getting merged?

Issue #72 is breaking a lot of code (namely, several examples of various dust-helpers) and I find the following particularly ugly:

var dust = require('dustjs-linkedin');
//require('dustjs-helpers');
dust.helpers = require('dustjs-helpers').helpers; //HACK
require('dustmotes-provide');

Thank you

[prashn64]

Still looks good. Do you think this should be a patch, minor, or major release? I was thinking minor, but can you think of anything backwards incompatible?

[kate2753]

I think minor bump is appropriate here since we've bumped up dust core from 2.3 to 2.4

This change is backward compatible with previous versions.