atticoos / gulp-ng-config

:wrench: Create AngularJS constants from a JSON config file
MIT License
173 stars 35 forks source link

Values of constans wrapped in double quotes breaks JSHint #12

Open jayrmotta opened 9 years ago

jayrmotta commented 9 years ago

A part of the strings on this file are written in single quotes and the other part is written in double quotes and it breaks JSHint validation.

Example:

angular.module('foo.config', [])
.constant('foo', "bar");
atticoos commented 9 years ago

Whoops! thanks

atticoos commented 9 years ago

For now, may be good to ignore the generated file in your jshint task.

jayrmotta commented 9 years ago

@ajwhite that's fine!

Here is my workaround until we fix it:

gulp.task('config', function() {
  var env = argv.env ? argv.env : 'development';

  gulp.src('conf/' + env + '.json')
  .pipe(gulpNgConfig('cockpitUi.config', {
    wrap: '(function () {\n\'use strict\';\n/*jshint ignore:start*/\n return <%= module %> /*jshint ignore:end*/\n})();'
  }))
  .pipe(rename('config.js'))
  .pipe(gulp.dest('src/app/'));
});

Just ignore the rename and env hack, I did that so I could change among many configs easily.

jasonswett commented 9 years ago

+1 for defaulting to single quotes. @jayrmotta's wrap solution worked for me.

atticoos commented 9 years ago

Placing on the roadmap

atticoos commented 9 years ago

Fixed in #29 by @psalaets :+1:

atticoos commented 9 years ago

Actually, just realized the PR was for double quotes, which broke your tests.

Going to reopen for further consideration. A possibility would be to configure singleQuotes: Boolean, or to add jshintIgnore: true

johannesjo commented 9 years ago

I would be OK with either one. Just would be nice to get rid of the warning for now.

singleQuotes: Boolean has the advantage that it would work not only with jshint but also with jscs and similiar tools though.

wembernard commented 9 years ago

Would it be that hard to transform "strings" to 'strings' during the JSON to JS process?

Adding JSHint rules sounds more like a temporary workaround than a true solution to me.

atticoos commented 9 years ago

@wembernard yep, we'll be going with the singleQuotes mode. It should not be that hard, just some escaping required. Feel free to open a PR

wembernard commented 9 years ago

@ajwhite I forked your project and managed to handle this by changing

_.each(jsonObj, function (value, key) {
  constants.push({
    name: key,
    value: JSON.stringify(value, null, spaces)
  });
});

to

_.each(jsonObj, function (value, key) {
  var processedValue;

  if (typeof value === 'boolean' || typeof value === 'number') {
    processedValue = value;
  } else {
    processedValue = JSON.stringify(value, null, spaces).replace(/"/g, '\'');
  }

  constants.push({
    name: key,
    value: processedValue
  });
});

However, I can't find a way to pass your gulp test.

atticoos commented 9 years ago

The outputs have changed, so all the tests will fail. This should only happen if single quote property is provided

porfidev commented 8 years ago

exist a method or function to change the doublequotes to singlequote after gulp-ng-config?

markmssd commented 8 years ago

Sounds pretty good to me! GJ!

Torone commented 7 years ago

Hi, any news about this? I don't think an option for singleQuote has been added and my generated file has double quote yet. Thanks

franbueno commented 7 years ago

Any news? Thanks! :)

Torone commented 7 years ago

Well, I just created a plugin that do the dirty job. You can find it here: https://www.npmjs.com/package/gulp-replace-quotes. It replace single quotes to double quotes or vice versa...