'grunt jshint:server' ignores deep server/api files

[bkapicka]

Are files nested in the server/api purposefully excluded from jshint:serve?

If not, proposed minor updates outlined below:

Updated Gruntfile:

src: [ 'server/{,*/}*.js']

to

src: [ 'server/**/*.js']

This causes the boilerplate specs to fail on 'grunt jshint:server'. Two additional changes to address:

1. Remove "latedef": true from Server .jshintrc
2. Add /*global describe, it, before, beforeEach, after, afterEach */
   and /*jshint expr:true */ to thing.spec.js and user.spec.js

[kingcody]

I agree with 2, but do you know the files/line numbers for the latedef issue?

[bkapicka]

@kingcody
Server .jshintrc template file link
File Path: generator-angular-fullstack / app / templates / server / .jshintrc
Line Number: L7.

[kingcody]

@bkapicka, sorry I actually meant: what were the offending files? The ones that had late definitions.

[bkapicka]

@kingcody my mistake - it was line 87 of the template thing.controller.js:
output from generated app:

server/api/thing/thing.controller.js
  line 66  col 21  'handleError' was used before it was defined.

[kingcody]

Ok, great. I'll see about getting a PR together to address that. Were there any other files that did not pass?

Btw; it's my 'opinion' that the less overrides in the linter the better, barring globals. Just my opinion though

[bkapicka]

Thanks for the PR.

And my apologies - missed two latedef errors. The full set of problems pre changes to jshintrc are below.

Re overrides: Completely agree but since the grunt jshint:server mixes specs and api files, it was the quickest solution to add /*jshint expr:true */ to the specs. Perhaps it is worth a separate grunt task for server specs requiring a new .jshintrc.

server/api/thing/thing.controller.js
  line 66  col 21  'handleError' was used before it was defined.

server/api/thing/thing.socket.js
  line 18  col 16  'onSave' was used before it was defined.
  line 22  col 18  'onRemove' was used before it was defined.

server/api/thing/thing.spec.js
  line 7   col 1   'describe' is not defined.
  line 9   col 3   'it' is not defined.

server/api/user/user.model.spec.js
  line 54  col 45  Expected an assignment or function call and instead saw an expression.
  line 58  col 45  Expected an assignment or function call and instead saw an expression.
  line 14  col 1   'describe' is not defined.
  line 15  col 3   'before' is not defined.
  line 22  col 3   'afterEach' is not defined.
  line 28  col 3   'it' is not defined.
  line 35  col 3   'it' is not defined.
  line 45  col 3   'it' is not defined.
  line 53  col 3   'it' is not defined.
  line 57  col 3   'it' is not defined.

[kingcody]

@bkapicka I think I see what you're saying. As in, it would not be advised to define describe, before, afterEach, it... as globals for non-spec files, to which I would agree.

@DaftMonk, @JaKXz any thoughts on this subject?

[JaKXz]

I agree with @bkapicka's points, but I don't think latedef should be removed from the .jshintrc. I think a PR to fix these issues would be great guys, thanks!

[kingcody]

@JaKXz I totally miss understood @bkapicka first point. I thought he was suggesting that we ADD latedef not remove it. I was under the impression it was NOT in the server .jshint. But I see now.

[DaftMonk]

I wouldn't mind getting rid of latedef. Those functions are hoisted, so there shouldn't be any issues with it.

It makes sense to me to have the public api of a module closer to the top for easier scanning of a file, and private methods near the bottom.

[kingcody]

@DaftMonk should we look at using latedef: nofunc? Just noticed it was an option. Not sure if that would be a good middle ground?

[DaftMonk]

@kingcody Nice find! That sounds perfect.