Sage Gulp Jshint - Trouble passing plugins

sage
1

Hi there,

I just wondered how everyone else had been getting along with ingesting extra javascript into there main.js file via watch and build tasks and jshint.

Because from where I am a the moment it’s been a real fight. Scripts that I routinely built minified with Roots and Grunt seem to output a large number of warnings and fail to build.

For example if I add the Velocity.js via bower Velocity.js
(adding it to my manifest.json)

{
“dependencies”: {
“main.js”: {
“files”: [
“…/bower_components/velocity/velocity.js”,
“scripts/main.js”
],
“main”: true

I get the following output.

/srv/users/serverpilot/apps/cmd/web/app/themes/sage/bower_components/velocity/velocity.js
  line 162   col 20   'id' is already defined.
  line 349   col 34   'offsetParent' is already defined.
  line 351   col 41   Confusing use of '!'.
  line 360   col 30   'offsetParent' is already defined.
  line 677   col 6    Unnecessary semicolon.
  line 725   col 22   Missing 'new' prefix when invoking a constructor.
  line 725   col 57   Missing 'new' prefix when invoking a constructor.
  line 729   col 26   Missing 'new' prefix when invoking a constructor.
  line 729   col 71   Missing 'new' prefix when invoking a constructor.
  line 736   col 43   Expected '{' and instead saw 'return'.
  line 784   col 37   Use '===' to compare with '0.0'.
  line 795   col 43   Expected '{' and instead saw 'calcSampleValues'.
  line 799   col 32   Expected '{' and instead saw 'precompute'.
  line 800   col 45   Expected '{' and instead saw 'return'.
  line 801   col 27   Expected '{' and instead saw 'return'.
  line 802   col 27   Expected '{' and instead saw 'return'.
  line 898   col 130  Unexpected use of '|'.
  line 1083  col 32   'i' is already defined.
  line 1309  col 21   Confusing use of '!'.
  line 1329  col 41   Missing 'new' prefix when invoking a constructor.
  line 1329  col 72   Missing 'new' prefix when invoking a constructor.
  line 1335  col 48   Missing 'new' prefix when invoking a constructor.
  line 1336  col 37   Expected a 'break' statement before 'case'.
  line 1354  col 77   Missing 'new' prefix when invoking a constructor.
  line 1370  col 41   Missing 'new' prefix when invoking a constructor.
  line 1374  col 44   Missing 'new' prefix when invoking a constructor.
  line 1377  col 22   Don't make functions within a loop.
  line 1386  col 28   'i' is already defined.
  line 1437  col 41   Confusing use of '!'.
  line 1458  col 22   Don't make functions within a loop.
  line 1549  col 31   Use '===' to compare with '0'.
  line 1639  col 21   Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function.
  line 1663  col 25   Missing 'new' prefix when invoking a constructor.
  line 1666  col 33   Missing 'new' prefix when invoking a constructor.
  line 1667  col 41   Missing 'new' prefix when invoking a constructor.
  line 1670  col 41   Missing 'new' prefix when invoking a constructor.
  line 1772  col 21   Missing 'new' prefix when invoking a constructor.
  line 1772  col 38   Missing 'new' prefix when invoking a constructor.
  line 1797  col 38   Expected '{' and instead saw 'console'.
  line 1828  col 37   Missing 'new' prefix when invoking a constructor.
  line 1856  col 63   Expected '{' and instead saw 'console'.
  line 1859  col 32   Missing 'new' prefix when invoking a constructor.
  line 1859  col 49   Missing 'new' prefix when invoking a constructor.
  line 1867  col 46   Expected '{' and instead saw 'console'.
  line 1882  col 83   Missing 'new' prefix when invoking a constructor.
  line 1885  col 17   Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function.
  line 1904  col 24   Missing 'new' prefix when invoking a constructor.
  line 1930  col 24   Missing 'new' prefix when invoking a constructor.
  line 1931  col 38   Missing 'new' prefix when invoking a constructor.
  line 1963  col 13   It's not necessary to initialize 'value' to 'undefined'.
  line 1963  col 13   Too many errors. (50% scanned).

  ✖  1 error
  ⚠  50 warnings

[06:46:11] 'jshint' errored after 4.49 s
[06:46:11] Error in plugin 'gulp-jshint'
Message:
    JSHint failed for: /srv/users/serverpilot/apps/cmd/web/app/themes/sage/bower_components/velocity/velocity.js

Am I doing something wrong? Or is jhint just harsh.

2

Hi,
i have had the same problem. I installed a extra javascript via bower install < package-name > -save and only got errors when implementing into manifest.json. For me it worked with adding nothing to manifest.json and just run gulp after bower install.

3

Trouble is that without it being added to the manifest the script won’t be minified into the dist/main.js file.

Running grunt we where able to build themes that routinely achieved PageSpeed Insights over 80 and this was in a large part down to the script combine and minification.

Anyone managed to use the current gulp setup to successfully minify scripts?

4

Hey guys,

Third party dependencies should not be placed in the “files”. Everything under the “files” property is considered first-party code and will be jshinted.

Furthermore: manually putting bower paths in the “files” property is not necessary. They are already included in the compiled output automatically via https://github.com/ck86/main-bower-files under the hood. So remove “…/bower_components/velocity/velocity.js” from your manifest.json.

To install something like Velocity.js just run

bower install velocity --save

And then recompile your scripts

gulp

Recommend you check out this simplified example: here

2 Likes
5

Thanks @austin sounds like I’ve just being going about this the wrong way for bower and not properly declaring the save.

But I do still wonder, how would we included 3rd party scripts for minification that aren’t available via Bower?

6

Include them via the vendor property

{
"dependencies": {
"main.js": {
"files": [
"scripts/main.js"
],
"vendor": [
"vendor/some/path/file.js"
],
"main": true
}
1 Like
7

Well I’ve given that a whirl and I think I’m half way there.

Bower install ‘dependancy’ –save was what I had missed for installing js dependencies.

But I’m still having trouble including files not brought in by Bower. I checked the documentation and vendor is certainly the prefix I should be using but, Gulp, Gulp Build and Gulp Watch don’t seem to be building this file into the final output.

    {
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "vendor": [
		"scripts/plugins/jquery.matchHeight.js"
	  ],
      "main": true
8

You know what? It’s probably because I intended vendor to be outside the assets folder. Try “assets/scripts/plugins/jquery.matchHeight.js”

I really need to document the “vendor” property better. https://github.com/austinpray/asset-builder/issues/25

1 Like
9

Hi @austin,

That little tweak got things working. Glad I wasn’t being to dim witted.

For our team it’s important for us to a be able to bower in dependancies and fall back on that pool of ‘old faithful’ scripts when necessary. If I had said to the team, ‘you must only use bower javascript’ I think I’d have got more than a few angry stares <.<

10

Yep that’s exactly what the vendor property is for!

You can also dequeue and include external plugin js with the vendor property as well.

11

That is great, I’ll keep the link to the documentation to this handy as I have a feeling this is where I will getting the most questions.

Thanks again ^-^

12

Thanks for the help, finally managed to get it working. :smile:

For those who would like to see the process I followed in detail please see this post: How to include custom/vendor scripts on gulp --production and avoid jshint errors

Hopefully that will help others too.

1 Like
13

Woooah unexpected i just facing this problem with some js today, and what i do is on the gulpfile.js i give my crazy hack on there in line 164
before

gulp.task('scripts', ['jshint'], function() {

i remove the jshint after and run gulp and its run right :wink:

gulp.task('scripts', [''], function() {

so i think this is really a bad hack right? because the gulp won’t screening every js i have with the jshint.

Ok i tried the vendor solution and its work but something bug me so for example i have

assets/vendor/all.js
assets/vendor/bootstrap.js

and on my manifest.json

"main.js": {
  "files": [
    "scripts/main.js"
  ],
  "vendor": [
    "vendor/bootstrap.js",
    "vendor/all.js"
  ],
  "main": true
},

then this mean is kinda like a rails asset pipeline that combine all of the js files into one file called main.js, right? so this is the best practice rather than have

<script src=".../main.js ...
<script src=".../bootstrap.js ...
<script src=".../all.js ...

isn`t it? if its right then this rule also apply to the css that combine every css into one main file