Dust helpers empty on server side require

[Tivoli]

With version 1.2 requiring helpers on the server side adds no helpers to the dust object. It does work fine on the client side.

dust = require('dustjs-linkedin')
require('dustjs-helpers')
console.log dust.helpers

This outputs {} when used on the server side only. 1.1.x will output the helpers in the object.

[kate2753]

Looking at the code it seems broken in both 1.1.x and 1.2.x. It was returning object with helpers in 1.0.x. Are you sure this works in 1.1.x ?

I will look into fixing this today or tomorrow.

[Tivoli]

Yep, just double checked, I'm on 1.1.2 currently.

[kate2753]

I just gave this a try and I'm having trouble reproducing issue in 1.2.0. This is how I tested tihs:

>mkdir helpers
>cd helpers
>npm install [email protected]
>npm install [email protected]
>node
>console.log(require('dustjs-helpers').helpers)

The output I get is

{ tap: [Function],
  sep: [Function],
  idx: [Function],
  contextDump: [Function],
  if: [Function],
  math: [Function],
  select: [Function],
  eq: [Function],
  ne: [Function],
  lt: [Function],
  lte: [Function],
  gt: [Function],
  gte: [Function],
  default: [Function],
  size: [Function] }

Am I missing something?

This is the code that makes sure helpers object is added onto module.exports. AFAIK it hasn't changed between 1.1.2 and 1.2.0.

(function(dust){
   ...
   var helpers = {
      ...
   };

   dust.helpers = helpers;
})(typeof exports !== 'undefined' ? module.exports = require('dustjs-linkedin') : dust);

[Tivoli]

Removed all my modules and reinstalled, seems to be working now, glitch in the matrix :P

[Tivoli]

Ok spoke too soon, so it seems like the helpers don't extend dust if required afterward anymore is the issue. But in the case of just requiring the helpers it does work.

> var dust = require("dustjs-linkedin")
undefined
> require('dustjs-helpers')
{ debugLevel: 'NONE',
  silenceErrors: false,
  log: [Function],
  onError: [Function],
  helpers: 
   { tap: [Function],
     sep: [Function],
     idx: [Function],
     contextDump: [Function],
     if: [Function],
     math: [Function],
     select: [Function],
     eq: [Function],
     ne: [Function],
     lt: [Function],
     lte: [Function],
     gt: [Function],
     gte: [Function],
     default: [Function],
     size: [Function] },
  cache: {},
  register: [Function],
  render: [Function],
  stream: [Function],
  renderSource: [Function],
  compileFn: [Function],
  load: [Function],
  loadSource: [Function],
  isArray: [Function: isArray],
  nextTick: [Function: nextTick],
  isEmpty: [Function],
  filter: [Function],
  filters: 
   { h: [Function],
     j: [Function],
     u: [Function: encodeURI],
     uc: [Function: encodeURIComponent],
     js: [Function],
     jp: [Function] },
  makeBase: [Function],
  escapeHtml: [Function],
  escapeJs: [Function],
  parse: [Function],
  compile: [Function],
  filterNode: [Function],
  optimizers: 
   { body: [Function: compactBuffers],
     buffer: [Function: noop],
     special: [Function: convertSpecial],
     format: [Function: nullify],
     reference: [Function: visit],
     '#': [Function: visit],
     '?': [Function: visit],
     '^': [Function: visit],
     '<': [Function: visit],
     '+': [Function: visit],
     '@': [Function: visit],
     '%': [Function: visit],
     partial: [Function: visit],
     context: [Function: visit],
     params: [Function: visit],
     bodies: [Function: visit],
     param: [Function: visit],
     filters: [Function: noop],
     key: [Function: noop],
     path: [Function: noop],
     literal: [Function: noop],
     raw: [Function: noop],
     comment: [Function: nullify],
     line: [Function: nullify],
     col: [Function: nullify] },
  pragmas: { esc: [Function] },
  compileNode: [Function],
  nodes: 
   { body: [Function],
     buffer: [Function],
     format: [Function],
     reference: [Function],
     '#': [Function],
     '?': [Function],
     '^': [Function],
     '<': [Function],
     '+': [Function],
     '@': [Function],
     '%': [Function],
     partial: [Function],
     context: [Function],
     params: [Function],
     bodies: [Function],
     param: [Function],
     filters: [Function],
     key: [Function],
     path: [Function],
     literal: [Function],
     raw: [Function] } }
> dust.helpers
{}
> 

[jimmyhchan]

dust.helpers was broken out into a separate module a bit ago. npm install dustjs-helpers

[Tivoli]

Yep I know, but if you require dustjs-linkedin and then require dustjs-helpers you end up with empty helpers.

[jimmyhchan]

whoops. sorry didn't read the whole issue. looking deeper.

[jimmyhchan]

Confirmed... but this should work. It should be the same as 'https://github.com/umdjs/umd/blob/master/jqueryPluginCommonjs.js'.

some temporary work arounds
var dust = require("dustjs-linkedin");
var helpers = require('dustjs-helpers').helpers;
dust.helpers = helpers;

this is what our jasmine test runner is sort of doing... but it really shouldn't be.

less verbose but probably even more fragile
var dust = require('dustjs-helpers');

end of the day, this is a bug that we need to fix.
The expectation is....

var dust = require("dustjs-linkedin")
require('dustjs-helpers')

test.ok(typeof dust.helpers.eq === 'function');

[kate2753]

@jimmyhchan I'm not sure it's possible to have dust be defined in your module as local variable and expect to be able to use it inside require('dustjs-helpers').

On the other hand having dust as a global variable works:

> dust = require("dustjs-linkedin");
> require('dustjs-helpers');
> console.log(dust.helpers)
{ tap: [Function],
  sep: [Function],
  idx: [Function],
  contextDump: [Function],
  if: [Function],
 math: [Function],
  select: [Function],
  eq: [Function],
  ne: [Function],
  lt: [Function],
  lte: [Function],
  gt: [Function],
  gte: [Function],
  default: [Function],
  size: [Function] }

[Tivoli]

I actually have it as a global and not working for some reason. I was doing it in coffeescript as a one liner.

global.dust = require('dustjs-linkedin') ; require('dustjs-helpers')

But since the helpers return the dustjs object I've just changed it to simply

global.dust = require('dustjs-helpers')

But the first version should still work and extend the current dust object.

[kate2753]

I've updated node's spec runner to test this. See #73

Are there other scenarios that we need to consider?

[jimmyhchan]

@kate2753 I wasn't suggested that we add dust as global. Our code should work regardless of what we name the output of require('dustjs-linkedin'). require should pull in the same version of dust every time you require it. Node caches everything.

Consider that this works:

/** lib/underbiz */
(function(_) {
  _.biz = 'biz';
})(typeof exports!== 'undefined' ? module.exports = require('underscore') : _);

$ node
> var u = require('underscore');
> require('./lib/underbiz')
> u.biz
'biz'

[kate2753]

@jimmyhchan I see what you mean by node caching require('dustjs-linkedin'). Now it makes sense how this should work with local variable (local variable would be a reference to dust object in require cache, require('dustjs-helpers') will update the same object that local variable is referencing).

I'm confused now why defining global dust variable works. Travis build passed for #73

We use the same code in dustjs-helpers as in your example. I'm wondering why it does not work with local variable:

(function(dust){
   ...
   var helpers = {
      ...
   };

   dust.helpers = helpers;
})(typeof exports !== 'undefined' ? module.exports = require('dustjs-linkedin') : dust);

[jimmyhchan]

@kate2753 found it. The last line of dust helpers

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

should be

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

if we want to have the output of dustjs-helper be dust with helpers then we would in addtion add

dust.helpers = helpers
module.exports = dust // add this line

[jimmyhchan]

Up for debate:
what should this repo return, helpers or dust or nothing?

/**return nothing*/
dust.helpers = helpers
// nothing here

/** in use*/
var dust = require('dustjs-linkedin');
var out = require('dustjs-helpers');
// out is meaningless;

vs.

/** return helpers*/
dust.helpers = helpers;
module.exports = helpers;

/** in use*/
var dust = require('dustjs-linkedin');
var helpers = require('dustjs-helpers');

test.ok(helpers.eq === dust.helpers.eq);

vs.

/** return helpers*/
dust.helpers = helpers;
module.exports = dust;

/** in use*/
var dust = require('dustjs-linkedin');
var dust2 = require('dustjs-helpers');

test.ok(dust === dust2);

[Tivoli]

I prefer the last option, it would mean that I only need to include the helpers instead of including core + helpers.

[kate2753]

Returning nothing or helpers would be backward incompatible. What would be the benefit of returning helpers object vs dust object? Is it worth backward incompatible change?

In all of these cases dust object in require cache will be altered. That's most likely how helpers code will be used. I guess return value does not matter much. I don't mind dustjs-helpers module returning dust object.

[jimmyhchan]

Agreed it's not worth the back-incompat change. This should be a pretty quick change to move module.exports from the last line to inside the closure. Any takers?

I believe we should still recommend folks use the more verbose

var dust = require('dustjs-linkedin');
require('dustjs-helpers');

With this, developers will have full control of which dustjs-linkedin and dustjs-helper versions they are using instead of relying on dustjs-helpers to bump its dependency. Also, iterations on dustjs-linkedin happen a lot more frequently then in the helpers code.

[kate2753]

+1 I think we should be more explicit when requiring core dust and dust helpers. How can we recommend this approach? Should this be added to README.md?

I'll update #73 to include module.exports move.

[Tivoli]

Would it be possible to do both? If the dust object exists, simply extend it and return the extended dust object. Or if I simply require the helpers then it requires the defined package version and returns it a new dust object.

This would be backwards compatible, allowing for someone to include whichever version of dust, or allow for a single package include.

Basically just moving the dust object to the front of the check vs always requiring on the server.

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

[jimmyhchan]

I think the current proposal will do both. With the subtle differences that
if you require only the helper the dust version is controlled by the helper
repo and not your app

[popham]

I've just run into this problem, and my temporary fix is to clobber dustjs-linkedin from the node_modules of dustjs-helpers. I've got a local dustjs-linkedin, as the following use case implies that one should:

var dust = require('dustjs-linkedin');
require('dustjs-helpers');

So line one loads dustjs-linkedin from my project's node_modules, then line two adds helpers to its private version of dustjs-linkedin.

I don't see anyother way to make that use case work beside removing the dependency from your package.json. Is that not a deal killer for this use case? I don't mind keeping the versions in sync on my own, but I can't imagine that this is tenable from the project's perspective.

[popham]

After a little more thought....

Looking at dustmotes-provide, for instance. If I require('dustmotes-provide') and require('dustjs-helpers'), then the helpers reside on different dusts. I don't think that third party helpers should need to require dustjs-helpers to be sure that this works. As it stands, a third party helper has to decide whether its consumers will want dustjs-helpers or just dustjs-linkedin (or impose on them to figure out how all of this works). The third party helper also needs to document how to use it, since it would vary case-by-case.

Is best practice something like:

var dust = require('dustjs-linkedin');
dust.helpers = require('dustjs-helpers').helpers;

But what now for third party stuff? For a single method like provide, dust.helpers['provide']=... works. For helper blobs, though, should these blobs get namespaced as a standard practice? If there were something stubbed on dust itself, one could arrive at something literate and consistent:

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

This is backward compatible and intuitive so long as nobody has named their helper "merge".

[skoranga]

+1 for the dust.helpers.merge solution.

[rragan]

Passing along another disgruntled user.

psteeleidem [9:10 AM]
Figured it out... "dustjs-helpers" just clears out all helpers and adds it own!!!!!! that was a terrible idea...

psteeleidem [9:10 AM]
I don't even know what code is loading dustjs-helpers to ensure that my code is loading after that code 😞

psteeleidem [9:11 AM]
that wasted a good hour of my time 😞

[patrick-steele-idem]

+1
Thanks for sharing my frustrations, Rich. It seems like a simple fix to update dustjs-helpers to merge in the helpers using code similar to the following:

dust.helpers.eq = function(chunk, context, bodies, params) { ... };
dust.helpers.ne = function(chunk, context, bodies, params) { ... };
// ...

Am I missing something?

[patrick-steele-idem]

Please accept the following Pull Request or let me know if any other changes need to be made:
#82

Thanks,
Patrick

[rragan]

A huge +1 on this. It cost me an hour last night tracing down just why dust.helpers was getting clobbered. It turns out that dust helpers loaded dustjs-linked in as a dependency in node and that copy was nuking the already defined helpers. Patrick's change should be totally safe as all he did was assign each helper directly to dust.helpers rather than to helpers and then assign dust.helpers=helpers; at the end thus clobbering any earlier user-defined helpers. Can we at least get this done? Just saw another victim on stack exchange: http://stackoverflow.com/questions/24418171/using-dustjs-helpers-with-kraken-js.

I think this manifests in some other issues over in dustjs-linked in also.