search

roots / sage

WordPress starter theme with Laravel Blade components and templates, Tailwind CSS, and a modern development workflow
https://roots.io/sage/
MIT License
12.73k stars 3.06k forks source link

assets.json randomly munged when running gulp --production #1398

Closed natebeaty closed 9 years ago

natebeaty commented 9 years ago

Just curious if anyone else has seen this. Running gulp --production will sometimes output an assets.json with an incomplete or mangled file list, such as this beauty:

{
  "editor-style.css": "editor-style-03c28a52.css",
  "main.css": "main-432a583a.css"
}query.js": "jquery-e5581654.js",
  "svg4everybody.js": "svg4everybody-e9de7a9b.js",
  "main.js": "main-f66739a9.js"
}

It's maddeningly inconsistent—running gulp --production repeatedly will sometimes work, then sometimes spit out a mangled file.

Before I dig further, just wanted to see if anyone else has run into this before.

austinpray commented 9 years ago

Sounds like race condition city. I've never personally gotten this but I'm trying to reproduce it. What does your environment look like? Also npm list --depth=0

natebeaty commented 9 years ago

Thanks for the quick response. Unfortunately I think this is going to be hard to reproduce. Though we have seen the issue on two separate computers in the office.

Not sure what all you need to know about my environment, happy to paste in anything that helps.

Our gulpfile.js: https://gist.github.com/natebeaty/d8b8a5b46a9e895d1e57 (there are some svg tasks added, but we saw the same behavior after replacing with default gulpfile.js)

manifest.json:

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "vendor": [
        "bower_components/history.js/scripts/bundled-uncompressed/html5/jquery.history.js",
        "bower_components/imagesloaded/imagesloaded.pkgd.js",
        "bower_components/fitvids/jquery.fitvids.js",
        "bower_components/slick-carousel/slick/slick.js",
        "bower_components/velocity/velocity.js",
        "bower_components/masonry/dist/masonry.pkgd.js"
      ],
      "main": true
    },
    "main.css": {
      "files": [
        "styles/main.scss"
      ],
      "main": true
    },
    "editor-style.css": {
      "files": [
        "styles/editor-style.scss"
      ]
    },
    "jquery.js": {
      "bower": ["jquery"]
    },
    "modernizr.js": {
      "bower": ["modernizr"]
    },
    "respond.js": {
      "bower": ["respond"]
    },
    "svg4everybody.js": {
      "bower": ["svg4everybody"]
    }
  },
  "config": {
    "devUrl": "sce.dev"
  }
}
$ npm list --depth=0
sage@8.0.0 /Users/nate/Sites/sce/web/app/themes/sce
├── asset-builder@0.4.1
├── browser-sync@2.3.1
├── cheerio@0.18.0
├── del@1.1.1
├── gulp@3.8.11
├── gulp-changed@1.2.1
├── gulp-concat@2.5.2
├── gulp-flatten@0.0.4
├── gulp-if@1.2.5
├── gulp-imagemin@2.2.1
├── gulp-jshint@1.9.4
├── gulp-less@3.0.1
├── gulp-load-plugins@0.8.1
├── gulp-pleeease@1.2.0
├── gulp-plumber@0.6.6
├── gulp-rename@1.2.0
├── gulp-rev@3.0.1
├── gulp-sass@1.3.3
├── gulp-sourcemaps@1.5.1
├── gulp-svg2png@0.3.0
├── gulp-svgmin@1.1.1
├── gulp-svgstore@5.0.0
├── gulp-uglify@1.1.0
├── imagemin-pngcrush@4.0.0
├── jshint-stylish@1.0.1
├── lazypipe@0.2.2
├── merge-stream@0.1.7
├── traverse@0.6.6
├── wiredep@2.2.2
└── yargs@3.5.4
QWp6t commented 9 years ago

For environment: What version of Node and NPM are you running? What OS are you on? Architecture (x86-32 or x86-64)?

Also, is there anything else about your environment that we should probably know? Are you doing this on a slow file system, like over a network or something?

The more info we have about your environment, the better, as it will help us replicate the issue so we can diagnose it.

natebeaty commented 9 years ago

I'm on a pretty new iMac (3.4 GHz Intel Core i5) on OS X 10.10.2. Node v0.12.0, npm 2.6.1. I believe Yosemite is all x86-64, no? Node was installed via Homebrew.

I have an SSD as my main drive over USB3, and am not doing any work over a network, so that shouldn't be an issue.

I'm not sure if this is worth a lot of effort if no other folks are experiencing it. I just wanted to throw it out there to see if it was something obvious to look for. However, I'm certainly open to offering anything else I can do to help diagnose.

austinpray commented 9 years ago

@natebeaty as an aside:

Your manifest.json can be rewritten as:

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "main": true
    },
    "main.css": {
      "files": [
        "styles/main.scss"
      ],
      "main": true
    },
    "editor-style.css": {
      "files": [
        "styles/editor-style.scss"
      ]
    },
    "jquery.js": {
      "bower": ["jquery"]
    },
    "modernizr.js": {
      "bower": ["modernizr"]
    },
    "respond.js": {
      "bower": ["respond"]
    },
    "svg4everybody.js": {
      "bower": ["svg4everybody"]
    }
  },
  "config": {
    "devUrl": "sce.dev"
  }
}

The main:true attribute means "every bower component that has not already been extracted should be included in main.js".

For instance if you have a bower.json that has (for example) 

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "main": true
    }
}

would give you a main.js file with these files concatenated together:

If you did:

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "main": true
    },
    "mypersonaljquery.js": {
      "bower": ["jquery"]
    }
  }
}

It would give you two files. A main.js file with these files concatenated together:

And a mypersonaljquery.js with these files

jackjgrimm commented 9 years ago

I'm not sure if my problem is related but I can't get my gulp --production to build the assets.json file. I figured it was something wrong that I'm doing because I'm new to this set up. (Also running a newer iMac with Yosemite, intalled node with package manager). My manifest.json file is original except the devurl. Any help would be greatly appreciated, I can't get my awesome theme live!

natebeaty commented 9 years ago

@austinpray thanks for the aside, we were wondering if that was the proper way to set up manifest.json.

However, when I remove the "vendor" section, I get javascript errors from History not being defined (and if I just add History.js to "vendor", then I got errors from Masonry calls).

When I just add History and Masonry to the "vendor" definition, it works as expected:

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "vendor": [
        "bower_components/history.js/scripts/bundled-uncompressed/html5/jquery.history.js",
        "bower_components/masonry/dist/masonry.pkgd.js"
      ],
      "main": true
    },
    "main.css": {
      "files": [
        "styles/main.scss"
      ],
      "main": true
    },
    "editor-style.css": {
      "files": [
        "styles/editor-style.scss"
      ]
    },
    "jquery.js": {
      "bower": ["jquery"]
    },
    "modernizr.js": {
      "bower": ["modernizr"]
    },
    "respond.js": {
      "bower": ["respond"]
    },
    "svg4everybody.js": {
      "bower": ["svg4everybody"]
    }
  },
  "config": {
    "devUrl": "sce.dev"
  }
}

I assume this is from the order of inclusion? Hard to determine exactly why it's failing. This is my main.js file if it helps diagnose: https://gist.github.com/natebeaty/1384423d4ee6a778a503

austinpray commented 9 years ago

@natebeaty History.js is not included without manual intervention because it has no "main" property in bower.json. https://github.com/browserstate/history.js/blob/14968aa3cf2a27335f71d26386de0a5c4073835d/bower.json

Please go get this PR accepted: https://github.com/browserstate/history.js/pull/384 or use overrides: https://github.com/ck86/main-bower-files#overrides-options

also: https://github.com/austinpray/asset-builder/blob/master/help/troubleshooting.md#some-bower-packages-not-being-injected

natebeaty commented 9 years ago

@austinpray Ok, that makes sense. I assume Masonry has the same issue.

This ended up working for me:

{
  "name": "sage",
  "homepage": "https://roots.io/sage/",
  "authors": [
    "Ben Word <ben@benword.com>"
  ],
  "license": "MIT",
  "private": true,
  "dependencies": {
    "modernizr": "2.8.2",
    "jquery": "1.11.2",
    "history.js": "~1.8.0",
    "imagesloaded": "~3.1.8",
    "fitvids": "~1.1.0",
    "respond": "~1.4.2",
    "svg4everybody": "~1.0.0",
    "slick-carousel": "~1.4.1",
    "masonry": "~3.2.2",
    "velocity": "~1.2.2"
  },
  "overrides": {
    "modernizr": {
      "main": "./modernizr.js"
    },
    "history.js": {
      "main": "./scripts/bundled-uncompressed/html5/jquery.history.js"
    },
    "masonry": {
      "main": "./dist/masonry.pkgd.js"
    }
  }
}

Coming back to the original issue, we consistently see the gulp --production issue of mangled or incomplete assets.json file creation across three different machines.

I'm on my home machine now (2012 Retina MBP, 2.9 GHz i7, 10.10.2, Flash drive, npm 2.5.1, node v.0.12.0) and I first got this 88 byte file:

{
  "editor-style.css": "editor-style-03c28a52.css",
  "main.css": "main-a5fd3c16.css"
}

But now I run it again and again and I keep getting the mangled version, unable to get a workable assets.json file that I can deploy:

$ gulp --production
[20:26:34] Using gulpfile ~/Firebelly/sce/web/app/themes/sce/gulpfile.js
[20:26:34] Starting 'clean'...
[20:26:34] Finished 'clean' after 24 ms
[20:26:34] Starting 'default'...
[20:26:34] Starting 'wiredep'...
[20:26:34] Starting 'jshint'...
[20:26:34] Starting 'fonts'...
[20:26:34] Starting 'images'...
[20:26:35] Finished 'default' after 1.91 s
[20:26:36] Finished 'wiredep' after 2.28 s
[20:26:36] Starting 'styles'...
[20:26:37] Finished 'jshint' after 2.94 s
[20:26:37] Starting 'scripts'...
[20:26:41] Finished 'fonts' after 7.36 s
[20:26:46] Finished 'scripts' after 9.08 s
[20:26:46] Finished 'styles' after 9.94 s
[20:26:47] gulp-imagemin: Minified 64 images (saved 23.59 kB - 1.1%)
[20:26:47] Finished 'images' after 13 s
[20:26:47] Starting 'build'...
[20:26:47] Finished 'build' after 5.41 μs

assets.json output:

{
  "editor-style.css": "editor-style-03c28a52.css",
  "main.css": "main-a5fd3c16.css"
}query.js": "jquery-e5581654.js",
  "svg4everybody.js": "svg4everybody-e9de7a9b.js",
  "main.js": "main-75812bfc.js"
}

At my work machine (faster iMac), this happens about 1/3 of the time, and I just have to keep running gulp --production until I get a non-mangled assets.json and then I can cap staging deploy.

Any ideas of other things to try?

Here's the node packages on the machine:

$ npm list --depth=0
sage@8.0.0 /Users/natebeaty/Firebelly/sce/web/app/themes/sce
├── asset-builder@0.4.1
├── browser-sync@2.3.1 extraneous
├── cheerio@0.18.0
├── del@1.1.1
├── gulp@3.8.11
├── gulp-changed@1.2.1
├── gulp-concat@2.5.2
├── gulp-flatten@0.0.4
├── gulp-if@1.2.5
├── gulp-imagemin@2.2.1
├── gulp-jshint@1.9.4
├── gulp-less@3.0.2
├── gulp-load-plugins@0.8.1
├── gulp-pleeease@1.2.0
├── gulp-plumber@0.6.6
├── gulp-rename@1.2.0
├── gulp-rev@3.0.1
├── gulp-sass@1.3.3
├── gulp-sourcemaps@1.5.1
├── gulp-svg2png@0.3.0
├── gulp-svgmin@1.1.1
├── gulp-svgstore@5.0.0
├── gulp-uglify@1.1.0
├── imagemin-pngcrush@4.0.0
├── jshint-stylish@1.0.1
├── lazypipe@0.2.2
├── merge-stream@0.1.7
├── traverse@0.6.6
├── wiredep@2.2.2
└── yargs@3.6.0
fullyint commented 9 years ago

Seems crazy, but I think dashes/hyphens in my filenames may have been the culprit in munging my assets.json on gulp --production. Munging is gone after I changed main-old-ie.less and main-old-ie.css TO mainOldIE.less and mainOldIE.css. But it has been a while and I didn't explore it much.

But... I still have editor-style.[less|css] and it doesn't cause problems -- probably blows my theory.

natebeaty commented 9 years ago

@fullyint I wondered if that could be it also. I just changed the one file with hyphens (editor-style.scss) to editorStyle.scss and still had the same behavior.

However, when I took out editor-style.scss entirely from manifest.json, it worked! We don't even really use that file, so this helps. It may also point to where the problem is occurring, perhaps in the styles section?

Every time I add a second scss file, even if it's just test.scss with a few simple rules, it causes the mangled assets.json.

fullyint commented 9 years ago

I tested more after realizing I hadn't run gulp --production all that much since I "got it working." Turns out editor-style.css DOES give me trouble.

@austinpray likes hard data... 

15 randomly intermingled trials
manifest.json with/without editor-style.css

with    editor-style.css:    Y x x x x x Y x Y Y x x x x x
without editor-style.css:    Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y

                     Key:    x = mangled    Y = correct

System

OSX 10.9.5, 3.6 GHz Intel Core 2 Duo (64 bit), 8 GB RAM, SSD (low on space, feeling a bit laggy) NPM 2.7.3, Node 0.12.0 - via NVM (Homebrew originally, but carefully researched how to completely remove and reinstall via NVM)

npm list --depth=0
sage@8.1.0 /Users/fullyint/sites/clientname/mysagetheme
├── asset-builder@1.0.2
├── browser-sync@2.4.0
├── del@1.1.1
├── gulp@3.8.11
├── gulp-changed@1.2.1
├── gulp-concat@2.5.2
├── gulp-flatten@0.0.4
├── gulp-if@1.2.5
├── gulp-imagemin@2.2.1
├── gulp-jshint@1.9.4
├── gulp-less@3.0.2
├── gulp-load-plugins@0.9.0
├── gulp-pleeease@1.2.0
├── gulp-plumber@1.0.0
├── gulp-rename@1.2.0
├── gulp-rev@3.0.1
├── gulp-sass@1.3.3
├── gulp-sourcemaps@1.5.1
├── gulp-stripmq@0.0.1
├── gulp-uglify@1.1.0
├── imagemin-pngcrush@4.0.0
├── jshint-stylish@1.0.1
├── lazypipe@0.2.2
├── merge-stream@0.1.7
├── pleeease@3.2.0
├── traverse@0.6.6
├── wiredep@2.2.2
└── yargs@3.6.0

manifest.json

3 stylesheets with "main": true, but browsers only load one due to conditional comments for IE.

{
  "dependencies": {
    "main.js": {
      "files": [
        "scripts/main.js"
      ],
      "main": true
    },
    "main.css": {
      "files": [
        "styles/main.less"
      ],
      "main": true
    },
    "mainIE9.css": {
      "files": [
        "styles/main.less"
      ],
      "main": true
    },
    "mainOldIE.css": {
      "files": [
        "styles/mainOldIE.less"
      ],
      "main": true
    },
    "editor-style.css": {
      "files": [
        "styles/editor-style.less"
      ]
    },
    "jquery.js": {
      "bower": ["jquery"]
    },
    "modernizr.js": {
      "bower": ["modernizr"]
    }
  },
  "config": {
    "devUrl": "http://example.dev"
  }
}
natebeaty commented 9 years ago

Following your key:

iMac (specs above)
w/    editor-style.css:    xYYxYYxYxxYxxxY
w/out editor-style.css:    YYYYYYYYYYYYYYY

MBP (Don't have it front of me, but this was the pattern last night)
w/    editor-style.css:    xxxxxx
w/out editor-style.css:    YYYYYY
fullyint commented 9 years ago

Probably a race condition. After more testing, I think filenames are irrelevant and it's just a race condition, like @austinpray said. Now I think get it. Sorry. I forgot gulp's virtue of processing asychronously. Yet, I didn't find much about race conditions over at gulp-rev, so I feel some doubts.

My project has nearly equal processing time for the styles and scripts tasks, so they occasionally writeToManifest at nearly the same time. So, when I removed editor-style.css from manifest.json (as in comment above), the styles task is faster, and I assume that avoided the race condition.

Replicated on sage. I achieved munged assets.json on a vanilla Sage clone by adding extra css files in manifest.json to achieve roughly equal processing time for styles and scripts. But it was rare. No other conditions I tested seemed related (permutations of filenames, re-using asset files in multiple blocks like in my manifest.json above, heavy concurrent processing load on machine, etc.). Munging won't happen every time. You have to run gulp --production handful of times to see it for any given setup of manifest.json. I guess @natebeaty and I lucked upon the right balance.

@natebeaty if you're dying to test more...

@austinpray Any potential in doing a single write to assets.json after styles and scripts are processed? I haven't tried, but I wonder if it could be implemented it like this...

gulp styles --production wouldn't write to manifest, but maybe nobody runs that command. Could conditionally write to manifest inside styles and scripts tasks when they're called on their own.

austinpray commented 9 years ago

@fullyint yep definitely a race condition.

Experimenting with https://github.com/OverZealous/run-sequence is giving me good results. Have to wrap up the work week today but will kill this this weekend.

natebeaty commented 9 years ago

@fullyint I did indeed experiment with adding several more (and removing to minimal # of) scripts and styles to manifest without much change in behavior.

Something odd I noticed however: if I add "main": true to editor-style.scss section, it works fine. If I take it out (the default), I get a corrupted assets.json file every time on my MBP.

For now we've just removed editor-style.scss from our site since we don't need to style the WP editor, and like you, now that it's Just Working, I've gone back to getting work done.

austinpray commented 9 years ago

ping https://github.com/orchestrator/orchestrator/issues/21

austinpray commented 9 years ago

@natebeaty @fullyint try this out: https://github.com/austinpray/roots/commit/b254f9dd9cadf71189b5d0d7117c1cd35d33e4dc

Works for me

natebeaty commented 9 years ago

Perfect, works for me! I was able to run gulp --production on my MBP and get a non-corrupted assets.json every time, even with editor-style.css added.

fullyint commented 9 years ago

Perfect for me too! Thank you @austinpray!

If I understand, styles and scripts are no longer allowed to run parallel. Does that increase processing time much? Here are processing times, alternating between the old and new gulpfile.js. 

        Time per trial (in seconds)        Mean
old:    21 22 23 22 21 20 21 22 21 21      21.4
new:    23 23 23 22 23 22 24 23 23 22      22.8

                 Difference in means:      1.4 seconds
                          New vs old:      6.5% slower

1.4 seconds isn't a big deal for the occasional build, but if someone has a much bigger build, 6.5% slower might be nice to avoid. Of course I can't say for sure that it would be 6.5% for everybody.

In my comment above, I tried to imagine an approach that keeps everything async, but runs a single rev.manifest() once styles and scripts are done (i.e., make styles and scripts dependencies for writeToManifest).

Thanks again for all your work on this!

austinpray commented 9 years ago

@fullyint Thank you for benchmarking!

I have done some benchmarking here: https://docs.google.com/spreadsheets/d/12ZiI3QhDWGdGGHMUU6YCfVNKLncwV3qwoKbNGBQbgjc/edit?usp=sharing

Setup

➜  roots git:(sanicthehedgehog) gulp -v
[09:27:47] CLI version 3.8.11
[09:27:47] Local version 3.8.11
➜  roots git:(sanicthehedgehog) node -v
v0.10.36

screenshot 2015-03-30 09 52 35

Benchmark command

for i in {1..10}; do time gulp --production --silent; done

Environments

parallel

Sage master before patch

blocking

New patch with js and styles in sequential order

Trials

Basic Sage

Sage out of the box

Heavy JS

Installed a bunch of bullshit from bower:

➜  roots git:(master) bower install --save slick.js bxslider-4 d3 lodash backbone async angular hammer.js impress.js ember angular-ui-router
{
  "modernizr": "2.8.2",
  "jquery": "1.11.2",
  "bootstrap": "3.3.4",
  "slick.js": "~1.4.1",
  "bxslider-4": "~4.2.3",
  "d3": "~3.5.5",
  "lodash": "~3.6.0",
  "backbone": "~1.1.2",
  "async": "~0.9.2",
  "angular": "~1.3.15",
  "hammer.js": "~2.0.4",
  "impress.js": "~0.5.3",
  "ember": "~1.11.0",
  "angular-ui-router": "~0.2.13"
}

Results

Basic Sage Heavy JS Grand Total
blocking 6.2498 16.5554 11.4026
parallel 6.1078 16.5737 11.34075
% difference 2.32% 0.11% 0.55%

So on my side I am not seeing much of a difference. Adding more stress to the build process actually lowered the perf difference between them on my side as well.

Recommendations

Even worst case scenario if there was a 10% difference I would still favor the stability of the sequential manifest writes. The time when speed matters is definitely during gulp watch and these changes do not affect those at all. I would happily wait longer for gulp to work as long as it completes predictably. I've waited for like 2 minutes for a cpp program to compile before :P

If you want to have your cake and eat it too: you can definitely build the runSequence array dependent on the build environment. If enabled.rev (--production) then run them sequentially else run them all in parallel. But that adds complexity to the build process.

fullyint commented 9 years ago

@austinpray Brilliant! Simply beautiful. Thank you so much for all your time. And for the new release!

I don't expect you to look at this further, but I tested more. I had this idea that parallel processing should be faster, and I was struggling to let it go. But my results came out consistent with yours. It just emphasizes to me of the importance of getting real data to test (and disprove) my assumptions.

I'll spare you the explanation why, but I wanted to test the condition in which both the JS and the CSS were super heavy (not just the JS). I also adjusted the gulpfile for a revised parallel approach in which all tasks run in parallel, but styles and scripts output separate manifest files that are merged into a single assets.json at the end.

No real speed difference between sequential approach released in 8.1.1 vs my revised parallel approach: 

              Basic Sage               Heavy JS              Heavy JS & CSS
        sequential  parallel     sequential  parallel     sequential  parallel
          16.749     13.387        48.389     46.244        71.067     70.632
          13.094     13.205        45.439     47.170        68.677     70.259
          13.114     13.008        44.961     46.662        69.394     70.280
          13.062     13.067        45.252     47.330        69.005     70.708
          13.122     13.281        46.141     45.898        69.081     70.719
          13.366     13.519        47.177     49.528        72.492     74.016
          16.275     13.003        48.450     45.840        72.341     70.405
          13.015     13.235        45.111     45.707        66.778     69.999
          13.243     13.066        46.964     47.797        69.161     71.553
          13.050     13.335        46.398     46.442        67.290     71.353

mean      13.809     13.210        46.428     46.861        69.528     70.992
diff                 -4.33%                   +0.93%                   +2.11%
dskvr commented 9 years ago

This problem still seems to occur randomly even with all mentioned fixes, having scripts randomly break do to the base configuration of a boilerplate is a rather frustrating environment to develop in. I would add more information, but everything I would mentioned is already in here. Has there been any additional progress on this repeated issue since this thread was concluded?

natebeaty commented 9 years ago

@dskvr fwiw, I have not seen a reoccurrence of the issue since the fix, and we have deployed dozens of times over several months, and on several projects. It was very much a problem before, and has been nonexistent since.

dskvr commented 9 years ago

K, good to know, I'll run a diff against master to see if I can find anything. Thanks for the quick reply

mAAdhaTTah commented 9 years ago

Also make sure your npm deps are updated.