Allow generated JS to define new angular module #28

Merged
merged 1 commit into from Jul 2, 2013

Conversation

Projects
None yet
5 participants
@sidwood
Contributor

sidwood commented Jun 7, 2013

This commit allows the module option to be an object literal as well as a string. When defined as an object literal we can specify that a new angular module should be defined.

ngtemplates:    {
  myapp:        {
    options:    {
      module: {
        name: 'templates',
        define: true
      }
    },
    src:        'src/views/**.html',
    dest:       'dist/templates.js'
  }
}

This generates the following.

angular.module('templates', []).run(['$templateCache', function($templateCache) {
  ...
}]);

Commit includes full test coverage and README updates. Let me know if you have any concerns or change requests.

@sidwood

This comment has been minimized.

Show comment Hide comment
@sidwood

sidwood Jun 7, 2013

Contributor

I've noticed that the compiler's parameter list is getting too long now with all the additions. It's currently at 6 parameters with this PR.

If you'd like I'd be happy to submit a refactoring. I'm thinking we could pass in an options object to the compiler. I was tempted to do that in this PR but I thought I should check with you first.

Contributor

sidwood commented Jun 7, 2013

I've noticed that the compiler's parameter list is getting too long now with all the additions. It's currently at 6 parameters with this PR.

If you'd like I'd be happy to submit a refactoring. I'm thinking we could pass in an options object to the compiler. I was tempted to do that in this PR but I thought I should check with you first.

@ericclemmons

This comment has been minimized.

Show comment Hide comment
@ericclemmons

ericclemmons Jun 7, 2013

Owner

An I the only one that changes "myapp" to "templates" to change the module name?

Ever since the original PR went in, I've always wondered why it deserved its own option when the task target name was always intended to indicate the module.

Perhaps it's a documentation issue? Or is there a legitimate reason?

The "define" option is a great addition, but I'm wondering, if at some point "module" is removed in favor of using the target name as originally intended, perhaps "define" should be separate from "module".

Which goes to your second point: the argument list is getting long enough to warrant a refactor, but I'd recommend until after this PR.

(Don't worry, I'll be quick about it! :P)

Thanks, and nice to see ya again Sid!

Owner

ericclemmons commented Jun 7, 2013

An I the only one that changes "myapp" to "templates" to change the module name?

Ever since the original PR went in, I've always wondered why it deserved its own option when the task target name was always intended to indicate the module.

Perhaps it's a documentation issue? Or is there a legitimate reason?

The "define" option is a great addition, but I'm wondering, if at some point "module" is removed in favor of using the target name as originally intended, perhaps "define" should be separate from "module".

Which goes to your second point: the argument list is getting long enough to warrant a refactor, but I'd recommend until after this PR.

(Don't worry, I'll be quick about it! :P)

Thanks, and nice to see ya again Sid!

@sidwood

This comment has been minimized.

Show comment Hide comment
@sidwood

sidwood Jun 7, 2013

Contributor

Maybe I'm missing something but it seems better to keep grunt build tasks and app module architecture separate. What if someone has different build requirements for the dev cycle vs. production build? For example, when testing directives with external templates it's nice to have the templates compiled in one location and then compiled in a different location when building for distribution.

Originally I thought of just adding defineModule: true to the options object but I noticed the denormalised prefixing and decided on a nested config object. If that's not your cup-o-tea I'd be happy to change it. Let me know.

It's the weekend here in London so I'm off to the pub :o)

Contributor

sidwood commented Jun 7, 2013

Maybe I'm missing something but it seems better to keep grunt build tasks and app module architecture separate. What if someone has different build requirements for the dev cycle vs. production build? For example, when testing directives with external templates it's nice to have the templates compiled in one location and then compiled in a different location when building for distribution.

Originally I thought of just adding defineModule: true to the options object but I noticed the denormalised prefixing and decided on a nested config object. If that's not your cup-o-tea I'd be happy to change it. Let me know.

It's the weekend here in London so I'm off to the pub :o)

@sidwood

This comment has been minimized.

Show comment Hide comment
@sidwood

sidwood Jun 8, 2013

Contributor

Another use case for multiple build targets using the same module name are i18n/i10n templates. Its feasible that someone would like to build a template cache for each locale. So the build process would A) generate the html templates using translation data and grunt template syntax, then B) use grunt-angular-templates to generate a 'templates' module for each locale. The end result would be multiple build artefacts, each an angular module with the name 'templates'. Does that make sense?

Contributor

sidwood commented Jun 8, 2013

Another use case for multiple build targets using the same module name are i18n/i10n templates. Its feasible that someone would like to build a template cache for each locale. So the build process would A) generate the html templates using translation data and grunt template syntax, then B) use grunt-angular-templates to generate a 'templates' module for each locale. The end result would be multiple build artefacts, each an angular module with the name 'templates'. Does that make sense?

@ericclemmons

This comment has been minimized.

Show comment Hide comment
@ericclemmons

ericclemmons Jun 8, 2013

Owner

Wow, that totally makes sense!

Thanks for explaining the reason to me, you're absolutely right.

I'll merge in this PR once I get home.

Thanks again!

Owner

ericclemmons commented Jun 8, 2013

Wow, that totally makes sense!

Thanks for explaining the reason to me, you're absolutely right.

I'll merge in this PR once I get home.

Thanks again!

@sidwood

This comment has been minimized.

Show comment Hide comment
@sidwood

sidwood Jun 10, 2013

Contributor

Nice one! Let me know how you want to proceed with the compiler refactor. If you're happy for me to handle it I can get to it later this week.

Contributor

sidwood commented Jun 10, 2013

Nice one! Let me know how you want to proceed with the compiler refactor. If you're happy for me to handle it I can get to it later this week.

@kozmic

This comment has been minimized.

Show comment Hide comment
@kozmic

kozmic Jun 17, 2013

+1

kozmic commented Jun 17, 2013

+1

@ericclemmons ericclemmons merged commit ce54c0c into ericclemmons:master Jul 2, 2013

1 check passed

default The Travis CI build passed
Details
@ericclemmons

This comment has been minimized.

Show comment Hide comment
@ericclemmons

ericclemmons Jul 2, 2013

Owner

Just tagged & released v0.3.9, thanks to you @sidwood!

I'm really sorry for the delay. Hectic few weeks for me. I just inherited ~15 new developers!

Owner

ericclemmons commented Jul 2, 2013

Just tagged & released v0.3.9, thanks to you @sidwood!

I'm really sorry for the delay. Hectic few weeks for me. I just inherited ~15 new developers!

@yaru22

This comment has been minimized.

Show comment Hide comment
@yaru22

yaru22 Jan 16, 2014

Contributor

Hi @ericclemmons, what happened to this define option? I need to create a new module but I don't know how to do it in v0.5.1.

Contributor

yaru22 commented Jan 16, 2014

Hi @ericclemmons, what happened to this define option? I need to create a new module but I don't know how to do it in v0.5.1.

@ericclemmons

This comment has been minimized.

Show comment Hide comment
@ericclemmons

ericclemmons Jan 16, 2014

Owner

Howdy @yaru22. https://github.com/ericclemmons/grunt-angular-templates#standalone If there was a section in the Examples, would you have caught that? (Dunno how to structure grunt plugin readmes in the most useful way...)

Owner

ericclemmons commented Jan 16, 2014

Howdy @yaru22. https://github.com/ericclemmons/grunt-angular-templates#standalone If there was a section in the Examples, would you have caught that? (Dunno how to structure grunt plugin readmes in the most useful way...)

@yaru22

This comment has been minimized.

Show comment Hide comment
@yaru22

yaru22 Jan 16, 2014

Contributor

Hi @ericclemmons. standalone option is exactly what I was looking for. I created #71 to add a brief explanation of that option. Maybe it can get better but it's a good start =)

Contributor

yaru22 commented Jan 16, 2014

Hi @ericclemmons. standalone option is exactly what I was looking for. I created #71 to add a brief explanation of that option. Maybe it can get better but it's a good start =)

@lautarobock

This comment has been minimized.

Show comment Hide comment
@lautarobock

lautarobock Mar 3, 2014

I'm using 0.5.3 but it doesnt work. put [object Object] as a module name

I'm using 0.5.3 but it doesnt work. put [object Object] as a module name

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