dust.load -> internal breaks users of the old API

[aredridel]

I was doing untoward hacks, replacing dust.load in dustjacket for internationalization purposes, and this being private now makes that impossible.

onLoad isn't rich enough for my purposes, which otherwise require doing unbelievably ugly things in the cache.

[aredridel]

(also: can have semver? Please?!)

[sethkinast]

Yeah... that's why dust.load has never been documented as being part of the API :P

If monkey-patching is inevitable, does it make sense to move it to the render layer or a dust.cache setter?

[sethkinast]

(The semver thing is some sort of legacy at LinkedIn that I haven't been able to push past-- FWIW Dust versioning is "minor adjusts API, major adjusts template API")

[sethkinast]

So what you really need is the ability to hot-plug different dust.onLoads? How can we make your life easier than doing horrible hacks?

[sethkinast]

(incidentally, just making load public again would still break you horribly as the internals have all changed, so we need to enhance whatever was making you need to monkeypatch load)

[sethkinast]

Ideally, I feel like you build your middleware into dust.onLoad-- middleware can then be provided a wrapped version of callback that does anything special you need before delegating to the real callback.

[aredridel]

Actually, after proving the concept, I was planning on trying to get dustjacket integrated, or something with its concepts. The specific goal I have is breaking the 1:1 template name to template mapping. The problem with doing it in onLoad is the caching behavior. You can't do internationalization that way, and having a central place to do the mapping from the language-agnostic render() calls to the specific templates is super useful, rather than pushing that complexity into all the call sites.

[aredridel]

Patching render might be the next best thing for me at this point -- it's the actual outside API where the name passed to load is used.

[sethkinast]

So template names are actually divorced from cache now.

You can compile a template with no name: var tmpl = dust.compile("Yo")

And render a template with no name: dust.render(tmpl, ctx, cb)

And register that template under any name you want... dust.register('superTemplate', tmpl)

Does it maybe make sense in your onLoad to blow away the default cache entry (assuming templates that get compiled with names) and re-register under a mangled i18n name?

My adaro-fu is weak and reading the code where middleware is added is not as useful probably as seeing it actually be used somewhere.

[aredridel]

Yeah, that's a possibility. not as clean as a stateless mapping though.

https://github.com/krakenjs/dustjacket/blob/v2.x/index.js is where the middleware gets added.

[aredridel]

At its core, though, the API is usually dust.render(templateName, ctx, cb), yes?

[sethkinast]

dust.render(templateNameOrFunction, ctx, cb)

So if it's a name, look it up in dust.cache, otherwise, just use whatcha got (dust.isTemplateFn can make sure it's a real template function)

Can you elaborate on how a single template name might result in two different templates? Can it result in different templates in the same instance of a running server? The name lookup is dynamic every time?

Is it something like load foo.js => I'm using fr_FR => load foo_fr_FR.js? Nothing gets cached? The lookup gets cached for next time? foo.js is now always the French version until a server restart?

[aredridel]

Yeah, the cache is used, but under the expanded name, not the shorter. Each new render does the lookup afresh.

dust.render('foo', { lang: "fr-FR"}, cb) renders fr-FR/foo, and dust.render('foo', {lang: 'en-US'}, cb) renders en-US/foo.

Invalid values of lang, or unsupported ones would render en-US/foo. Because it allows defaults.

[aredridel]

Actually just looked at the new load ... my monkey patch would actually still work. It maintains the same outside contract and replaces load wholesale.

[sethkinast]

Short-term, it seems to make sense to have a dust.onLoad wrapper that mutates the template name before delegating to the real dust.onLoad. onLoad is the supported point for hooking into loading shenanigans. Alternatively, dust.render is the public API... even if monkeypatches are gross, sometimes you gotta do what you need to.

Going forward, I think enhancing dust.onLoad to traverse a middleware chain, and passing it the context as well to make it context-aware, sounds pretty cool.

[aredridel]

I'll start working that direction then. Context-awareness first, that's the core point. And letting onLoad return a function so that it can manage caching.

[aredridel]

Oh, the only problem there: dust.onLoad can't mutate the template name.

[sethkinast]

Hmm, you're right.

dust.render seems to tick all the boxes then. Has context, can adjust template name before calling load, is public API.

[sethkinast]

OK, as a less-drastic first step here's what I'm working on:

Currently dust.onLoad has two behaviors: either you load a precompiled template that registers itself and call cb(), or you pass uncompiled template source to cb(null, src) and it compiles and registers under the requested name.

I'm going to add a third signature, which is cb(null, tmpl), where you pass a compiled template. The template doesn't get registered under the requested name. That way you can register foo_fr_FR and load it from cache yourself if you want to.

I hope that will be the first step to making your life a little easier.

[aredridel]

Absolutely! I was actually preparing a pull request to that effect just today -- I'd complete that tomorrow if you want to leave it to me.

[aredridel]

(only slow part was learning the intricacies of the test suite)

[sethkinast]

Oop I immediately started writing after I posted without checking for replies.

I'll put up what I have so far instead of finishing it and you can take a look.

[sethkinast]

https://github.com/linkedin/dustjs/pull/641/files feel free to incorporate or not and let's make sure this fits your use case. It won't be the end-all, but maybe a good stopgap.

[aredridel]

Cool. Lemme see!

[sethkinast]

Closed in #641