Skip to content

Add `commonjs` option. #43

Closed
wants to merge 1 commit into from

3 participants

@spikebrehm

This adds a commonjs option, which allows exporting compiled templates as a CommonJS module, following the example set by the amd option.

What do you guys think?

@tkellen
grunt member
tkellen commented Apr 2, 2013

Sorry for the slow reply here--I'm holding off until gruntjs/grunt-contrib-pug#19 is complete.

@tkellen
grunt member
tkellen commented Jul 14, 2013

Sorry, I am mostly out of commission on this stuff until the construction season is over--my wife and I are in the middle of building an off-grid homestead.

Can someone else from the team step in on this stuff?

/cc @cowboy, @geddski, @shama, @sindresorhus, @jsoverson etc

@shama shama and 1 other commented on an outdated diff Jul 14, 2013
@@ -28,7 +28,7 @@
"test": "grunt test"
},
"dependencies": {
- "handlebars": "~1.0.10",
+ "handlebars": "git://github.com/spikebrehm/handlebars.js.git#6bbcf3f2225b0e62d249481d07456698da7da65e",
@shama
grunt member
shama added a note Jul 14, 2013

Why not the actual handlebars package here?

@spikebrehm
spikebrehm added a note Jul 15, 2013

Oops, yes; I should have made the PR from a branch, so additional commits to master didn't creep in. I'll fix that.

In case you're curious, I was pointing to a Handlebars commit that included support for CommonJS out of the box (wycats/handlebars.js#537), which has been merged into their master, but a new version has not been published to NPM.

@shama
grunt member
shama added a note Jul 15, 2013

Ok cool. I'll keep watch on this thread. Let's wait until he publishes the changes as it sounds like he will soon. Thanks for the PR btw!

@spikebrehm
spikebrehm added a note Jul 15, 2013

No reason to wait on wycats' changes; the issue of bundling Handlebars itself using a CommonJS loader (besides browserify, which is already supported) is orthogonal to the changes in this PR, which allow templates to be packaged as CommonJS modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@spikebrehm

@shama I force-pushed to remove those two extra commits. This should be kosher now.

Also LMK if it's bad practice to update the version to 0.5.9 and the changelog in the PR, in case you would like to manage that separately from the code change itself.

@shama
grunt member
shama commented Jul 15, 2013

@spikebrehm Awesome thanks! I'm watching wycats/handlebars.js#537 as well and will follow up on this as soon as they publish. Feel free to ping me if you don't mind as well.

About bumping for maintainers... It depends on the maintainer. I personally don't mind when people do that. Thanks man!

@shama shama closed this in 5465926 Jul 15, 2013
@shama
grunt member
shama commented Jul 15, 2013

@spikebrehm Oh you're right, this change doesn't depend on handlebars publishing. Merged and published as 0.5.10. Thanks!

@spikebrehm

Sweet! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.