shannonmoeller / gulp-hb

A sane Gulp plugin to compile Handlebars templates. Useful as a static site generator.
http://npm.im/gulp-hb
MIT License
147 stars 14 forks source link

Possibility to explicitly pass data instead of data file paths #10

Closed backflip closed 9 years ago

backflip commented 9 years ago

Would you consider adding a possibility to pass a data object instead of just paths? Main use cases:

I realize I could add the data to the file object and use the fileoption, however, I would have to namespace all references in my templates with file. or file.data. which would make them less readable.

anpieber commented 9 years ago

I've a different problem which could be solved by the same solution.

I try to create multiple different output files based on the same input file but with different data objects. This makes the way glob works with data objects completely insufficient for this use case. Since I really like the plugin otherwise and after seeing this issue I thought that it might be a good idea providing a pull request around this problem.

The provided pull request adds an additional test and keeps up the 100% code coverage. No linting errors could be found. Let me know if you need anything else.

shannonmoeller commented 9 years ago

Thanks for the issue and pull request! I'll take a closer look at both this afternoon.

backflip commented 9 years ago

I really like the solution proposed by @anpieber.

shannonmoeller commented 9 years ago

Because I'm the author of the primary dependencies, I implemented an alternate solution further up the dependency chain and released the bumped version as v2.2.0. You can now specify an object literal for data, partials, and helpers. Or, a function that returns any of those things. Check out the README and tests for more info and let me know what you think!

vs.src(config.templates + 'objects.html')
    .pipe(hb({
        file: false,
        data: {
            foo: require('./fixtures/data/foo'),
            bar: require('./fixtures/data/bar.json'),
            users: require('./fixtures/data/users.json')
        },
        helpers: {
            lower: require('./fixtures/helpers/lower'),
            upper: require('./fixtures/helpers/upper'),
            'flow-when': require('./fixtures/helpers/flow/when')
        },
        partials: function () {
            var name = condition ? 'foo' : 'bar';

            return {
                'components/item': require('./fixtures/partials/components/' + name + '.hbs')
            };
        }
    }))
    .pipe(...);
shannonmoeller commented 9 years ago

One thing I didn't implement was the change to how the files flag works. What do you guys thing about changing the flag to accept an alternate name? Similar to the property option of gulp-front-matter.

backflip commented 9 years ago

Thanks for the update, this works for me.

Adding a property option sounds reasonable. An alternative would be to pass the file as an argument if data is a function:

data: function(file) {
    return file.frontMatter;
}

This would probably be the most flexible approach.

backflip commented 9 years ago

On second thought: This would probably be necessary in my case since every template in the stream needs specific data. Unfortunately, I did not realize this when creating the issue.

What do you think about checking whether data is a function and not passing it through to require-glob in this case?

module.exports = function (options) {
    // …

    // Find and merge all data
    if (options.data && typeof options.data !== 'function') {
        data = requireGlob.sync(options.data, {
            cwd: cwd
        });
    }

    // …

    // Stream it. Stream it good.
    return map(function (file, cb) {
        // …

        if (options.data && typeof options.data === 'function') {
            context = options.data(file);
        }

        // …
    });
};
anpieber commented 9 years ago

@shannonmoeller Thank you very much for the solution. It works perfectly!

shannonmoeller commented 9 years ago

@backflip The primary use case that I'm solving for is "render many handlebars files using the same options." So, I'd like to avoid rerunning functions and globbing for each file in the pipe for performance reasons. You can now pass a function to the data property, but it will only run once.

Your use case is "render a single handlebars file using different options." I've created an example gulpfile for you to consider.

backflip commented 9 years ago

The performance of the default behavior should still be the same:

The example provided would work in some cases, however, I can't rely on every template having it's own data file. So I do have to work on a stream of templates instead.

But I certainly understand if you don't want to address this specific use case. We already have our own solution for this (https://github.com/unic/gulp-unic-handlebars). I just didn't want to publish yet another plugin before making sure that existing plugins don't already solve the same problem.

shannonmoeller commented 9 years ago

Oh, I see what you're saying. In that instance, I'd generally reach for gulp-data as their very first example does exactly what you're looking for. However, that brings us back full-circle to the file.data namespace issue. I'm guessing that using a {{#with file.data}} block wrapper in your templates is also undesirable?

backflip commented 9 years ago

Exactly, I wouldn't really like that.

backflip commented 9 years ago

Possible solution/hack: Adding a property option which could be set to an empty string which would lead to the file object being directly merged into the context. Okay, not really.

shannonmoeller commented 9 years ago

Okay. I'm not a fan of blocking the current functionality of options.data so what would you say to an options.dataEach callback that would be run on a per-file basis? That way users can glob/define global data once up front, then modify as desired on a per-file basis. That would solve my performance concerns.

backflip commented 9 years ago

Hm, I don't think I know the complete data set because I'm requiring the data for each template while passing it through the stream:

gulp.src(paths.templates)
    .pipe(tap(function(file) {
        var dataFile = util.replaceExtension(file.path, '.data.js'),
            data = require(dataFile);

        file.data = data;
    }))
    .pipe(handlebars({
        data: function(file) {
            return file.data;
        },
        partials: paths.partials
    }))
    .pipe(rename({
        extname: '.html'
    }))
    .pipe(gulp.dest(paths.dest));
shannonmoeller commented 9 years ago

Exactly, so in your case you'd use dataEach instead of data and leave data blank.

    .pipe(handlebars({
        dataEach: function(file) {
            return file.data;
        },
        partials: paths.partials
    }))
backflip commented 9 years ago

Ah, sorry, I thought dataEach would only modify data already provided with the data option. But this sounds exactly like what I need.

shannonmoeller commented 9 years ago

Addition of dataEach option published as v2.3.0. Thanks, @backflip and @anpieber !

shannonmoeller commented 9 years ago

@backflip FYI, the callback signature is (context, file), so your example case would be:

    .pipe(handlebars({
        dataEach: function(context, file) {
            return file.data;
        },
        partials: paths.partials
    }))
backflip commented 9 years ago

Awesome, thanks a lot!

backflip commented 7 years ago

@shannonmoeller Any chance of re-adding this? It's currently not possible to assign file-specific data to @root (which seems to be useful, see https://github.com/unic/estatico/issues/36)

shannonmoeller commented 7 years ago

@backflip I've opened #49 to track this rather than reopen this old issue.