linkedin / dustjs

Asynchronous Javascript templating for the browser and server
http://dustjs.com
MIT License
2.91k stars 478 forks source link

Prioritize resolution by `.then` for thenable functions #735

Closed brianmhunt closed 8 years ago

brianmhunt commented 8 years ago

Here are a couple simple changes to make function-thenables work as expected and increase compatibility with e.g. Knockout:

1. Change dust.isThenable to

  dust.isThenable = function(elem) {
    return elem &&
           (typeof elem === 'object' || typeof elem === 'function') &&
           typeof elem.then === 'function';
  };

This matches Promises A+ spec, which states "1.2 “thenable” is an object or function that defines a then method."

(As a total aside, you may want to test that elem !== null since typeof null is object and null.then will throw an exception.)

2. Towards the end of Context.prototype._get change:

So if a "thenable function" (a function with a .then) is passed in, it prioritizes resolution by via the .then over calling the function.

3. In Chunk.prototype.reference move:

if (dust.isThenable(elem)) {
  return this.await(elem, context, null, auto, filters);
}

to the beginning of the function (i.e. before if (typeof elem === 'function') {).

4. Add a test, along these lines (pardon the style/naming):

describe("references ...", function () {
  it("dereferences via .then on thenable functions", function () {
    var fn = function () { return 'no' }
    fn.then = (res) => "yes"
    dust.renderSource("a{x}b", {x: fn}, (err, out) => assert.equal(out, "ayesb"))
  })
})

Use case

If you are using Knockout.js (and lots of folks do to) then most view-models passed in will use knockout observables, and look like this:

var vm = {
   x: ko.observable()
}

These observables are functions; you call them to read the value, write to them by passing the new value as the argument to the same function i.e. x(123) sets the value to 123; x() then returns 123.

When you call dust with this with e.g. dust.renderSource("{x}", vm, ...) it modifies the observable, which is undesirable. (It becomes the chunk via ctx.apply(ctxThis, arguments); in Context.prototype._get.)

Prioritizing thenable over functions makes it trivial for Knockout to expose its value. (For interests sake, until it's built in, KO users need to add something like this: ko.subscribable.fn.then = function(res) { res(this()) }). I also feel like this is the more natural prioritization.

I doubt many folks will have thenable functions, so I suspect this change has a low probability of breakage – and where folks do have thenable functions I suspect the preference is to resolve via the then as opposed to a function call.

Thanks for reading; I hope this proves to be a useful request. Happy to put together a PR, but I just posted here since the changes seem pretty simple.

Cheers.

brianmhunt commented 8 years ago

I just noted at least one other place where a small change is required, namely Chunk.prototype.section. Will look for and post about others.

sethkinast commented 8 years ago

Interesting, thanks so much for the thorough overview. I see you're working on a PR; I'm totally on-board with pulling this in when you're ready.

sethkinast commented 8 years ago

As an aside, how does one use Knockout with Dust, anyways? I would have assumed that Knockout handled all the templating.

brianmhunt commented 8 years ago

Awesome. Thank you so much for receiving and considering the PR, which is now up as #736.

As an aside, how does one use Knockout with Dust, anyways? I would have assumed that Knockout handled all the templating.

Knockout is essentially template-agnostic, but it's much easier to stick with the default templating system. The data/observable model, however, can be completely severed from Knockout.†

† I am in the process of breaking Knockout out into reusable parts now (per, among others, tko.observable).