brandingbrand/express-cdn — pull request #8 by nkm8 (2014-06-30 17:38:24+00:00)
Title: PCJS-991 - Fix missing variable declaration that resulted in a hoisitng/...

...race condition when uploading image assets to S3

https://brander.atlassian.net/browse/PCJS-991

Behavior:
Intermittently, when checkout.styl is dynamically compiled, background-image properties in either checkout.css or main.css will have a value of 'true', which breaks the sprites.

To reproduce:
Run a project (Party City) locally in production mode. Then, make a change to either checkout.styl or base.styl. Restart front-end and view the compiled CSS files on S3 (filenames are output in console) and do a search for 'true' - there should be several results in either checkout.css or main.css.

Root cause:
The images array is undeclared. So, we end up with a hoisiting/race condition that is based on the order the assets are compiled (which is system-dependent). Either 1 of checkout.styl or main.styl will be compiled first, the images array is populated, the imagesToUpload array is populated, and the async.parallel request is started and the image files are uploaded one at a time (takes a decent amount of time to comlete).

During the upload, immediately, the next .STYL file is compiled. During the next call to checkStringIfModified, imagesToUpload is reset to be empty, but the images array contains the data from the previous .STYL file. Since the placeholder for the images array is 'true', nothing is ever added to the imagesToUpload array (for duplicate images, which is usually the case here).

When we call async.parallel with an empty imagesToUpload array, the final callback fires immediately. Based on the race condition with the previous uploads that were initiated, when we do the body.replace call, the value replaced in the background-image property is either going to be the correct S3 URL (previous upload luckily finished already) or 'true' (previous upload not yet complete).

To ensure that all images are uploaded successfully before doing the final body.replace assignment, we must reset the images array, which will correctly populate the imagesToUpload array.

This fix ensures that all image assets required for each .STYL file are uploaded independently and are set in the resulting .CSS file only when all the assets for that file are successfully uploaded.