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

Values of constans wrapped in double quotes breaks JSHint #12

Open
jayrmotta opened this issue Jan 14, 2015 · 17 comments
Open

Values of constans wrapped in double quotes breaks JSHint #12

jayrmotta opened this issue Jan 14, 2015 · 17 comments
Labels

Comments

@jayrmotta
Copy link

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
Copy link
Owner

Whoops! thanks

@atticoos atticoos added the bug label Jan 31, 2015
@atticoos
Copy link
Owner

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

@jayrmotta
Copy link
Author

@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
Copy link

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

@atticoos atticoos added this to the v1.3.0 milestone May 1, 2015
@atticoos
Copy link
Owner

atticoos commented May 1, 2015

Placing on the roadmap

@atticoos
Copy link
Owner

Fixed in #29 by @psalaets 👍

@atticoos
Copy link
Owner

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

@atticoos atticoos reopened this Jun 26, 2015
@johannesjo
Copy link

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
Copy link

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
Copy link
Owner

@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
Copy link

@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
Copy link
Owner

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

@atticoos atticoos removed this from the v1.3.0 milestone Nov 17, 2015
@porfidev
Copy link

porfidev commented Jun 13, 2016

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

seyDoggy pushed a commit to seyDoggy/gulp-ng-config that referenced this issue Sep 9, 2016
Activity: atticoos/issues/12

I ran into [this issue](atticoos/issues/12) as well where linters (Sonarlint in my case) take
exception to the double-quotes output. I've changed the template and
test files to use single-quotes and am using .replace on the
template.constant.value to replace double-quotes with single-quotes.

Tests pass.
@markmssd
Copy link

Sounds pretty good to me! GJ!

@Torone
Copy link

Torone commented Nov 29, 2016

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
Copy link

Any news? Thanks! :)

@Torone
Copy link

Torone commented Jan 29, 2017

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants