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

Already on GitHub? Sign in to your account

Configure path to underscore in AMD wrapper (option 1 of 2) #20

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

colinhicks commented Feb 21, 2013

Addresses #18. See discussion there for context.

Since compiled templates include a reference to _, that dependency needs to be explicitly defined when the AMD wrapper is used.

With this pull, given amdWrapper: true, the first line of jst output is:

define(['underscore'], function(_) {

The default path to the underscore module can be overriden in the Gruntfile (see option 2 for alternate syntax):

// Gruntfile.js
jst: {
    compile {
        options: {
          amdWrapper: true,
          amdPathToUnderscore: 'path/to/lodash'
        },
        files: {
          "build/templates.js": ["templates/**/*.html"]
        }
    }
}

// build/templates.js
define(['path/to/lodash'], function(_) {

Note: You may also choose to specify this path in your RequireJS config.

Owner

tkellen commented Feb 22, 2013

Hey Colin, thanks for taking the time to put this PR (and the other) together. I'd like to merge this, but I think having the dependency name be 'underscore' is sufficient, given that you can override this in your RequireJS config.

@tbranyen / @lazd would you agree?

If not, I'll merge with the option to name the underscore module, if so, I'll merge after that has been removed.

Owner

tkellen commented Feb 26, 2013

Owner

vladikoff commented Oct 19, 2013

hey @tkellen this is one of the old PRs in the gruntjs org, what should we do about it?

Owner

tkellen commented Oct 20, 2013

Defining underscore as a dependency on the AMD wrapper is fine. I don't think we need to support amdPathToUnderscore, though. You can configure the path to lodash/underscore in your RequireJS config.

@colinhicks would you be willing to update this PR?

Member

sindresorhus commented Dec 11, 2013

ping, also need to fix merge conflict.

@vladikoff vladikoff closed this Jan 20, 2014

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