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

Option for coffeescript output, switch deprecated grunt.util._ to lodash #11

Merged
merged 1 commit into from Nov 1, 2013

Conversation

jjt
Copy link
Contributor

@jjt jjt commented Oct 28, 2013

Coffeescript output is false by default, can be enabled with options.coffee. Uses js2coffee to convert the built javascript string to coffeescript before saving the file to disk.

Also, as per Grunt's docs, using grunt.util._ has been deprecated in favour of installing lodash or underscore through npm and saving it in package.json.

@mlegenhausen
Copy link
Owner

Thanks for your push request. This feature looks handy even if I don't understand why you translate from js to coffee and then back again ;) An use case would be nice.

I will take a look later today.

@jjt
Copy link
Contributor Author

jjt commented Oct 28, 2013

No problem!

I'm using Yeoman with generator-angular, and it just seemed easiest to generate a coffee file in my app/scripts/services/ directory alongside all of my other coffee files and let generator-angular take care of things from there. That way I don't have to duplicate the dev/production directory configuration, and everything just works.

It's possible there was an easier way to integrate this with generator-angular, but here we are!

@mlegenhausen
Copy link
Owner

So you check the generated file in with your other source? I don't use generator-angular but wouldn't it be possible to put the generated file somewhere in .tmp and the generator-angular grunt file can pick it up from there to generate a dist build? I will take a look at it.

@mlegenhausen
Copy link
Owner

Ok I integrated it in the generator-angular workflow. I made following steps:

  1. Add ngconstant to your Gruntfile.js:
ngconstant: {
      dist: {
        dest: '.tmp/scripts/constants.js', // I think this is important
        name: 'constants',
        constants: {
          'DATA': [
            'HTML5 Boilerplate',
            'AngularJS',
            'Karma'
          ]
        }
      }
    }
  1. Add ngconstant to the tasks. They should look like this:
grunt.registerTask('server', function (target) {
  if (target === 'dist') {
    return grunt.task.run(['build', 'connect:dist:keepalive']);
  }

  grunt.task.run([
    'clean:server',
    'ngconstant',
    'concurrent:server',
    'autoprefixer',
    'connect:livereload',
    'watch'
  ]);
});

grunt.registerTask('test', [
    'clean:server',
    'ngconstant',
    'concurrent:test',
    'autoprefixer',
    'connect:test',
    'karma'
]);

grunt.registerTask('build', [
    'clean:dist',
    'ngconstant',
    'useminPrepare',
    ...
]);
  1. Fix your karma configuration:
files: [
      'app/bower_components/angular/angular.js',
      'app/bower_components/angular-mocks/angular-mocks.js',
      'app/bower_components/angular-resource/angular-resource.js',
      'app/bower_components/angular-cookies/angular-cookies.js',
      'app/bower_components/angular-sanitize/angular-sanitize.js',
      '.tmp/scripts/*.js',  // Add these two lines
      '.tmp/scripts/**/*.js',
      'app/scripts/*.js',
      'app/scripts/**/*.js',
      'test/mock/**/*.js',
      'test/spec/**/*.js'
],
  1. Add the new file to your index.html:
<!-- build:js({.tmp,app}) scripts/scripts.js -->
<script src="scripts/app.js"></script>
<script src="scripts/constants.js"></script>
<script src="scripts/controllers/main.js"></script>
<!-- endbuild -->
  1. Add the module constants to you app.js dependency list.
  2. Use it for example in your controller:
angular.module('yeomanAngularTestApp')
  .controller('MainCtrl', function ($scope, DATA) {
    $scope.awesomeThings = DATA;
  });

I hope this help. I would think the coffee script workflow should be the same.

@ghost ghost assigned mlegenhausen Oct 28, 2013
@jjt
Copy link
Contributor Author

jjt commented Oct 28, 2013

No, my config.LOCAL.coffee is ignored. I usually set up *.LOCAL.[extension(s)] in my .gitignore for each project so that it's easy to ignore stuff.

Well, this is what I have now with my fork of ng-constant:

    ngconstant: {
      options: {
        coffee: true
      },
      dist: {
        dest: 'app/scripts/services/config.LOCAL.coffee',
        name: 'localConfig',
        constants: {
          'rootUrlWeb': 'http://production.url/',
          'rootUrlApi': 'http://production.url/api/'
        }
      },
      dev: {
        dest: 'app/scripts/services/config.LOCAL.coffee',
        name: 'localConfig',
        constants: {
          'rootUrlWeb': 'http://dev.url/',
          'rootUrlApi': 'http://dev.url/api/'
        }
      }
    }
// ...

  grunt.registerTask('server', function (target) {
    if (target === 'dist') {
      return grunt.task.run(['build', 'open', 'connect:dist:keepalive']);
    }
    // Development server
    grunt.task.run([
      'clean:server',
      'ngconstant:dev', // <-- added this
// ...
  // Prod build
  grunt.registerTask('build', [
    'clean:dist',
    'ngconstant:dist',  // <-- added this

Both ways work, but IMO this is a cleaner, more Gruntish way. You define an ngconstant task with targets, and add them to the watch/build tasks as you see fit and everything else is the same in the build process without editing any other files. That means less deviation from the way that generator-angular sets everything out. Which gives an easier upgrade path and better troubleshooting due to being closer to vanilla installs of generator-angular (which has been downloaded over 15,000 times in the last month!).

@mlegenhausen
Copy link
Owner

Hmm I am not a fan of mixing up generated code with handwritten. But wouldn't it not work out of the box when changing the paths to .tmp/scripts/services/config.LOCAL.js. The only step more is to add the .tmp files to the karma configuration and of course can you use your different targets. I just used one for testing purpose.

Of course do you need to add the ngconstant task to your server configuration. I forgot this in my previous comment.

@jjt
Copy link
Contributor Author

jjt commented Oct 28, 2013

Fair point about mixing generated code with source. It doesn't bother me in my use case, as I'm just defining one module.

Yeah, it would work just fine the way you describe. I still think that the example I'm using is slightly less disruptive to the generator-angular system, and requires less knowledge of its workings.

I think the difference between our methods is minor, and either would work just fine.

@mlegenhausen
Copy link
Owner

Ok, this is your choice.

Then I have to talk about if ngconstant is the right place for such an integration or you shouldn't better use a separate grunt plugin like grunt-js2coffee which does excatly what you want. The integration would be as simple as the ngconstant integration by simply converting the file after you have created it.

@jjt
Copy link
Contributor Author

jjt commented Oct 29, 2013

My pull request just has to do with outputting coffeescript instead of javascript for any build process. My example was just one of many situations where it could be useful for someone.

Yes, using grunt-js2coffee would work, but as a user it's another plugin to configure and maintain in my Gruntfile instead of just passing in an option to ngconstant.

@jjt
Copy link
Contributor Author

jjt commented Oct 31, 2013

Just wondering, are you considering accepting this pull request? You initially said that it looks like a handy feature to have. I'd like to move to the mainstream grunt-ng-constant package available on npm, rather than maintaining my own fork.

If you have any issues with the PR, please let me know.

@mlegenhausen
Copy link
Owner

Sorry for no further feedback I am currently a little bit busy. I will merge this feature and will take a look at the code in the following days maybe already today.

@jjt
Copy link
Contributor Author

jjt commented Nov 1, 2013

Okay, I'll sit tight. Thanks.

mlegenhausen pushed a commit that referenced this pull request Nov 1, 2013
@mlegenhausen mlegenhausen merged commit c025d1d into mlegenhausen:master Nov 1, 2013
@mlegenhausen
Copy link
Owner

I made a little change to the configuration. You don't need the options block. So it is consistent with the other parameters. I will open a separate issue this will make the options configuration more consistent.

@jjt
Copy link
Contributor Author

jjt commented Nov 4, 2013

Great, thanks for merging this in. I just installed v0.4.5 from npm and everything works the way it should.

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.

None yet

2 participants