Merge branch 'hershmire-bug/post-processors-race-condition'

index.js

@@ -104,6 +104,7 @@ function Cartero( entryPoints, outputDirPath, options ) {
 
 	this.assetsRequiredByEntryPoint = {};
 	this.metaDataFileAlreadyWrited = false;
+	this.postProcessorTasks = [];
 
 	this.watching = false;
 
@@ -310,7 +311,7 @@ Cartero.prototype.processMains = function( callback ) {
 	var tempJavascriptBundleEmitter = new EventEmitter();
 
 	tempJavascriptBundleEmitter.setMaxListeners( 0 ); // don't warn if we got lots of listeners, as we need 1 per entry point
-	
+
 	function createTempJsBundleStreamsByEntryPoint() {
 		tempJsBundlesByEntryPoint = _.map( _this.mainPaths, function( thisEntryPoint ) {
 			var thisJsBundlePath = _this.getTempBundlePath( 'js' );
@@ -336,32 +337,59 @@ Cartero.prototype.processMains = function( callback ) {
 		}
 	} );
 
-	if( this.watch ) {
-		browserifyInstance.on( 'update', function() {
-			async.parallel( [ function( nextParallel ) {
-				browserifyInstance.bundle( function( err, buf ) {
-					if( err ) {
-						delete err.stream; // gets messy if we dump this to the console
-						log.error( '', err );
-						return;
-					}
+	var parallelTasks = [
+		function checkBrowserifyBundleDoneTask(nextParallel) {
+			browserifyInstance.bundle( function( err, buf ) {
+				if( err ) {
+					delete err.stream; // gets messy if we dump this to the console
+					log.error( '', err );
+					return;
+				}
+				commonJsBundleContents = buf;
+				nextParallel();
+			});
+		},
 
-					commonJsBundleContents = buf;
-					nextParallel();
-				} );
-			}, function( nextParallel ) {
-				var numberOfBundlesWritten = 0;
+		function checkParcelifyDoneTask(nextParallel) {
+			p.on('done', nextParallel);
+		},
 
-				tempJavascriptBundleEmitter.on( 'tempBundleWritten', function( thisMainPath, tempBundlePath ) {
-					numberOfBundlesWritten++;
+		function checkEntryPointBundlesDoneTask(nextParallel) {
+			var numberOfBundlesWritten = 0;
 
-					// don't have to do anything here... we are just waiting until all of our
-					// temp bundles have been written before moving on.
+			tempJavascriptBundleEmitter.on('tempBundleWritten', function(thisMainPath, tempBundlePath) {
+				numberOfBundlesWritten++;
+				// don't have to do anything here... we are just waiting until all of our
+				// temp bundles have been written before moving on. see below comments
+				if (numberOfBundlesWritten === _this.mainPaths.length) {
+					nextParallel();
+				}
+			});
+		},
 
-					if( numberOfBundlesWritten === _this.mainPaths.length ) nextParallel();
-				} );
-			} ], function( err ) {
+		// Make sure all postProcessing bundles have finished since it may complete
+		// after all entry point js bundles resolve.
+		function checkPostProcessingBundlesDoneTask(nextParallel) {
+			var counter = 0;
+			if (_this.postProcessors.length === 0) {
+				return nextParallel();
+			}
 
+			function _postProcessedFinishedHandler(event) {
+				counter++;
+				if (counter === _this.postProcessorTasks.length) {
+					_this.removeListener('postProcessedBundleFinish', _postProcessedFinishedHandler);
+					return nextParallel();
+				}
+			}
+			_this.removeListener('postProcessedBundleFinish', _postProcessedFinishedHandler);
+			_this.on('postProcessedBundleFinish', _postProcessedFinishedHandler);
+		}
+	]
+
+	if( this.watch ) {
+		browserifyInstance.on( 'update', function() {
+			async.parallel(parallelTasks, function( err ) {
 				_this.writeAllFinalJavascriptBundles( tempJsBundlesByEntryPoint, needToWriteCommonJsBundle ? commonJsBundleContents : null, function() {
 					// done
 				} );
@@ -370,32 +398,7 @@ Cartero.prototype.processMains = function( callback ) {
 	}
 
 	// in parallel, let parcelify and browserify do their things
-	async.parallel( [ function( nextParallel ) {
-		browserifyInstance.bundle( function( err, buf ) {
-			if( err ) {
-				delete err.stream; // gets messy if we dump this to the console
-				log.error( '', err );
-				_this.emit( 'error', err);
-				return;
-			}
-
-			commonJsBundleContents = buf;
-			nextParallel();
-		} );
-	}, function( nextParallel ) {
-		p.on( 'done', nextParallel );
-	}, function( nextParallel ) {
-		var numberOfBundlesWritten = 0;
-
-		tempJavascriptBundleEmitter.on( 'tempBundleWritten', function( thisMainPath, tempBundlePath ) {
-			numberOfBundlesWritten++;
-
-			// don't have to do anything here... we are just waiting until all of our
-			// temp bundles have been written before moving on. see below comments
-
-			if( numberOfBundlesWritten === _this.mainPaths.length ) nextParallel();
-		} );
-	} ], function( err ) {
+	async.parallel(parallelTasks, function(err) {
 		if( err ) return callback( err );
 
 		// we have to make sure that parcelify is done before executing this code, since we look up
@@ -415,7 +418,7 @@ Cartero.prototype.processMains = function( callback ) {
 		} );
 	} );
 
-	p.on( 'packageCreated', function( newPackage ) {
+	p.on('packageCreated', function( newPackage ) {
 		if( newPackage.isParcel ) {
 			_this.parcelsByEntryPoint[ newPackage.mainPath ] = newPackage;
 		}
@@ -561,6 +564,12 @@ Cartero.prototype.copyTempBundleToFinalDestination = function( tempBundlePath, a
 			} ); } );
 
 			if( postProcessorsToApply.length !== 0 ) {
+				// Keep track of all not script bundles if we have post processors to
+				// apply. This is so we know when these bundles are resolved.
+				if (assetType !== 'script') {
+					_this.postProcessorTasks.push(finalBundlePath);
+				}
+
 				// apply post processors
 				bundleStream = bundleStream.pipe( combine.apply( null, postProcessorsToApply.map( function( thisPostProcessor ) {
 					return thisPostProcessor( finalBundlePath );
@@ -569,6 +578,12 @@ Cartero.prototype.copyTempBundleToFinalDestination = function( tempBundlePath, a
 
 			bundleStream.pipe( fs.createWriteStream( finalBundlePath ).on( 'close', function() {
 				fs.unlink( tempBundlePath, function() {} );
+
+				// Notify that a postProcessed bundle has finished
+				if (_this.postProcessorTasks.indexOf(finalBundlePath) > -1) {
+					_this.emit('postProcessedBundleFinish');
+				}
+
 				_this.emit( 'fileWritten', finalBundlePath, assetType, true, this.watching );
 
 				callback( null, finalBundlePath );
@@ -620,7 +635,7 @@ Cartero.prototype.writeCommonJavascriptBundle = function( buf, callback ) {
 
 			if( this.watching && oldBundlePath ) {
 				// if there is an old bundle that already exists, delete it. this
-				// happens in watch mode when a new bundle is generated. (note the old bundle 
+				// happens in watch mode when a new bundle is generated. (note the old bundle
 				// likely does not have the same path as the new bundle due to sha1)
 				fs.unlinkSync( oldBundlePath );
 			}

test/css-post-processor-stream.js

@@ -0,0 +1,26 @@
+var through = require('through');
+var path = require('path');
+
+module.exports = function (file, options) {
+	var data = '';
+	var options = options || {};
+	if (file !== undefined && path.extname(file) !== '.css') {
+		return through();
+	} else {
+		return through(write, end);
+	}
+
+	function write(buffer) {
+		data += buffer;
+	}
+
+	function end() {
+		var that = this;
+		// Simulate the stream to take some time to end so we can test for any race
+		// conditions.
+		setTimeout(function() {
+			that.queue(data);
+			that.queue(null);
+		}, 2000);
+	}
+};

test/example5/static/.gitignore

@@ -0,0 +1 @@
+assets/

test/example5/views/common.css

@@ -0,0 +1,3 @@
+.boop {
+	font-size: 24px;
+}

test/example5/views/common.js

@@ -0,0 +1,2 @@
+
+console.log( 'got common' );

test/example5/views/package.json

@@ -0,0 +1,5 @@
+{
+	"name" : "common-js",
+	"main" : "common.js",
+	"style" : "*.css"
+}

test/example5/views/page1/package.json

@@ -0,0 +1,5 @@
+{
+	"view" : "page1.jade",
+	"main" : "page1.js",
+	"style" : "*.css"
+}

test/example5/views/page1/page1.css

@@ -0,0 +1,3 @@
+body {
+	background : blue;
+}

test/example5/views/page1/page1.js

@@ -0,0 +1,3 @@
+require( "../common" );
+
+console.log( 'hellooo dave' );

test/example5/views/page2/package.json

@@ -0,0 +1,5 @@
+{
+	"view" : "page2.jade",
+	"main" : "page2.js",
+	"style" : "*.css"
+}

test/example5/views/page2/page2.css

@@ -0,0 +1,3 @@
+body {
+	background : black;
+}

test/example5/views/page2/page2.jade

@@ -0,0 +1,7 @@
+doctype html
+html(lang="en")
+	head
+		| !{cartero_js}
+		| !{cartero_css}
+	body
+		h1 Boop

test/example5/views/page2/page2.js

@@ -0,0 +1,3 @@
+require( "../common" );
+console.log( 'hellooo dave' );
+

test/test.js

@@ -4,6 +4,7 @@ var test = require( 'tape' );
 var fs = require( 'fs' );
 var crypto = require( 'crypto' );
 var _ = require( 'underscore' );
+var cssPostProcessorStream = require('./css-post-processor-stream.js');
 
 var outputDirFiles = [ 'metaData.json' ];
 
@@ -32,7 +33,7 @@ test( 'example1', function( t ) {
 		);
 
 		t.deepEqual( JSON.parse( fs.readFileSync( path.join( outputDirPath, 'metaData.json' ), 'utf8' ) ).packageMap, packageMap );
-	
+
 		t.deepEqual(
 			fs.readdirSync( path.join( outputDirPath, packageId ) ).sort(),
 			[ 'page1_bundle_9238125c90e5cfc790e8a5ac8926185dfb162b8c.css', 'page1_bundle_08786d2274344b392803ce9659e6d469ede96834.js' ].sort()
@@ -140,7 +141,7 @@ test( 'example3', function( t ) {
 		);
 
 		t.deepEqual( JSON.parse( fs.readFileSync( path.join( outputDirPath, 'metaData.json' ), 'utf8' ) ).packageMap, packageMap );
-	
+
 		var page1PackageFiles = fs.readdirSync( path.join( outputDirPath, parcelIdsByPath[ 'page1' ] ) ).sort();
 		var page2PackageFiles = fs.readdirSync( path.join( outputDirPath, parcelIdsByPath[ 'page2' ] ) ).sort();
 
@@ -222,3 +223,62 @@ test( 'example4', function( t ) {
 
 	} );
 } );
+
+
+test( 'example5 – postProcessors', function( t ) {
+	t.plan( 4 );
+
+	var entryPoints = [ path.join( __dirname, 'example5/views/**/*' ) ];
+	var entryPointFilter = function( fileName ) {
+		return path.extname( fileName ) === '.js';
+	};
+	var outputDirPath = path.join( __dirname, 'example5/static/assets' );
+	var packageMap = {};
+	var packageIds = [];
+	var parcelIdsByPath = {};
+	var commonJsPackageId = '';
+	var c = cartero(entryPoints, outputDirPath, {
+		entryPointFilter: entryPointFilter,
+		assetTypes: ['style'],
+		postProcessors: [function(file) {
+	    return cssPostProcessorStream(file);
+		}]
+	});
+
+	c.on( 'packageCreated', function( newPackage ) {
+		if( newPackage.package.name === "common-js" )
+			commonJsPackageId = newPackage.id;
+
+		if( newPackage.isParcel ) {
+			var parcelId = newPackage.id;
+			parcelIdsByPath[ path.relative( path.join( __dirname, 'example5/views' ), newPackage.path ) ] = parcelId;
+		}
+
+		packageMap[ newPackage.path.slice(1) ] = newPackage.id;
+		packageIds.push( newPackage.id );
+	} );
+
+	c.on( 'done', function() {
+		t.deepEqual(
+			fs.readdirSync( outputDirPath ).sort(),
+			packageIds.concat( outputDirFiles ).concat( [ 'common_39ebe84e9d92379be5bdf9b8d938b38677a1d620.js' ] ).sort()
+		);
+
+		t.deepEqual( JSON.parse( fs.readFileSync( path.join( outputDirPath, 'metaData.json' ), 'utf8' ) ).packageMap, packageMap );
+
+		var page1PackageFiles = fs.readdirSync( path.join( outputDirPath, parcelIdsByPath[ 'page1' ] ) ).sort();
+		var page2PackageFiles = fs.readdirSync( path.join( outputDirPath, parcelIdsByPath[ 'page2' ] ) ).sort();
+
+		var page1CssBundle = _.find( page1PackageFiles, function( thisFile ) { return path.extname( thisFile ) === '.css'; } );
+		page1CssBundle = path.join( outputDirPath, parcelIdsByPath[ 'page1' ], page1CssBundle );
+
+		var page1CssContents = fs.readFileSync( page1CssBundle, 'utf8' );
+		t.ok( page1CssContents.indexOf( 'background : blue' ) !== -1, 'page 1 has correct background color' );
+
+		var page2CssBundle = _.find( page2PackageFiles, function( thisFile ) { return path.extname( thisFile ) === '.css'; } );
+		page2CssBundle = path.join( outputDirPath, parcelIdsByPath[ 'page2' ], page2CssBundle );
+
+		var page2CssContents = fs.readFileSync( page2CssBundle, 'utf8' );
+		t.ok( page2CssContents.indexOf( 'background : black' ) !== -1, 'page 2 has correct background color' );
+	} );
+} );