Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Node.js require() handler for .dust files #330

Closed
wants to merge 1 commit into from

Conversation

jjclark1982
Copy link

When working on server code with templates in individual .dust files, it is convenient to be able to require() those files into useful modules, like so:

var dust = require("dustjs-linkedin");
var helloTemplate = require("./templates/hello");
helloTemplate({name: "Nemo"}, function(err, result) {
    if (err) throw err;
    console.log(result);
});

This patch provides a require() handler to make that possible.

@hashanp
Copy link

hashanp commented Dec 30, 2013

+1

@jjclark1982
Copy link
Author

After using this for a while in production, I have found that it is useful to make require('file') provide access to the compiled dust block and the stream function as well as the render function.

@rragan
Copy link
Contributor

rragan commented Dec 30, 2013

How does this change relate to this other piece of work I saw recently?

https://github.com/michaelleeallen/dustjs-require

@jjclark1982
Copy link
Author

That is for AMD-style modules, this is for node-style modules. It is mostly useful for making dust templates available by the same syntax in node and in a CommonJS frontend.

I have been using it to load templates using relative filenames, so that a template file can be stored in the same directory as the view that consumes it, and the view doesn't need any knowledge of the directory structure in order to say template = require("./template")

@jimmyhchan
Copy link
Contributor

This is something we should have. Can I assume it is backwards compatible? If someone was referring to a template with an extension will they need to change their code?

I know there are a few other bug fixes in the 2.3.X milestone which may have priority over this.

@jimmyhchan jimmyhchan mentioned this pull request Jan 8, 2014
@prashn64
Copy link
Contributor

prashn64 commented Jan 9, 2014

@jjclark1982 Can you update your code to the latest?

I believe this is good enough to merge.

@jjclark1982
Copy link
Author

I am on honeymoon for the next week so it is hard to push code directly. Paging @aazlant to this thread.

@aaronazlant
Copy link

new code should look like this:

// Publish a Node.js require() handler for .dust files
if (require.extensions) {
  require.extensions[".dust"] = function(module, filename) {
      var fs = require("fs");
      var text = fs.readFileSync(filename, 'utf8');
      var source = dust.compile(text, filename);
      var tmpl = dust.loadSource(source, filename);

      module.exports = tmpl;
      module.exports.render = function (context, callback) {
        dust.render(filename, context, callback)
      }
      module.exports.stream = function (context) {
        dust.stream(filename, context, callback)
      }
  };
}

@jjclark1982
Copy link
Author

This handler is backwards-compatible with older versions of node and dust: it won't conflict with other ways of loading templates. Although it will publish the template under its full filename even if required with a shorter path, so some aliasing in 'dust.cache' is necessary to make the best use of memory while still allowing sub views to be loaded with their shortest names via '{>subview}'

Separating the template, render function and stream function like this is very forwards-compatible: it leaves an obvious place to add things like a 'reload()' function for cache invalidation during development. And it leaves the template block function available for onLoad manipulation.

@semanio
Copy link

semanio commented Dec 6, 2014

@jjclark1982 can you fix the merge conflicts on this? This has been re-mentioned recently per #516 as a potential solution.

@jjclark1982
Copy link
Author

Sure, I'll resolve the conflicts.

@jjclark1982
Copy link
Author

I am not sure how to add this to the test suite, but here is the code I used to test this functionality:

var template = require("../test/fixtures/example.dust");
var context = {name: "Somebody"};
template.render(context, function(err, result){
    assert(err === null);
    assert.equal(result, "Hello, Somebody!");
});

Notably missing from this implementation is a way to require a .dust file from another .dust file. I was able to achieve that with in-band signaling (a require() handler that set a $parentTemplate value in the context of the body it produces, coupled with an onLoad handler that consults that value), but such an approach is probably not suitable for upstream.

@sethkinast sethkinast removed this from the Milestone 2.4 - fast follow for 2.3 milestone Mar 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants