8
0
Просмотр исходного кода

Fixed error handling by... read the comments ;).

Piotrek Koszuliński 10 лет назад
Родитель
Сommit
1f8603752c
3 измененных файлов с 104 добавлено и 24 удалено
  1. 79 3
      dev/tasks/gulp/build.js
  2. 24 20
      dev/tasks/gulp/utils.js
  3. 1 1
      src/path.js

+ 79 - 3
dev/tasks/gulp/build.js

@@ -132,15 +132,91 @@ module.exports = ( config ) => {
 	gulp.task( 'build:clean', tasks.clean );
 
 	gulp.task( 'build', [ 'build:clean' ], () => {
+		//
+		// NOTE: Error handling in streams is hard.
+		//
+		// Most likely this code isn't optimal, but it's a result of 8h spent on search
+		// for a solution to the ruined pipeline whenever something throws.
+		//
+		// Most important fact is that when dest stream emits an error the src stream
+		// unpipes it. Problem is when you start using tools like multipipe or gulp-mirror,
+		// because you lose control over the graph of the streams and you cannot reconnect them
+		// with a full certainty that you connected them correctly, since you'd need to know these
+		// libs internals.
+		//
+		// BTW. No, gulp-plumber is not a solution here because it does not affect the other tools.
+		//
+		// Hence, I decided that it'll be best to restart the whole piece. However, I wanted to avoid restarting the
+		// watcher as it sounds like something heavy.
+		//
+		// The flow looks like follows:
+		//
+		// 1. codeStream
+		// 2. inputStream
+		// 3. conversionStream (may throw)
+		// 4. outputStream
+		//
+		// The input and output streams allowed me to easier debug and restart everything. Additionally, the output
+		// stream is returned to Gulp so it must be stable. I decided to restart also the inputStream because when conversionStream
+		// throws, then inputStream gets paused. Most likely it's possible to resume it, so we could pipe codeStream directly to
+		// conversionStream, but it was easier this way.
+		//
+		// PS. The assumption is that all errors thrown somewhere inside conversionStream are forwarded to conversionStream.
+		// Multipipe and gulp-mirror seem to work this way, so we get a single error emitter.
 		const formats = options.formats.split( ',' );
 		const codeStream = tasks.src.all( options.watch )
 			.on( 'data', ( file ) => {
 				gutil.log( `Processing '${ gutil.colors.cyan( file.path ) }'...` );
 			} );
-		const formatPipes = formats.reduce( utils.addFormat( distDir ), [] );
+		const converstionStreamGenerator = utils.getConversionStreamGenerator( distDir );
 
-		return codeStream
-			.pipe( gulpMirror.apply( null, formatPipes ) );
+		let inputStream;
+		let conversionStream;
+		let outputStream = utils.noop();
+
+		startStreams();
+
+		return outputStream;
+
+		// Creates a single stream combining multiple conversion streams.
+		function createConversionStream() {
+			const formatPipes = formats.reduce( converstionStreamGenerator, [] );
+
+			return gulpMirror.apply( null, formatPipes )
+				.on( 'error', onError );
+		}
+
+		// Handles error in the combined conversion stream.
+		// If we don't watch files, make sure that the error is forwarded to the output.
+		// If we watch files, then clean up the old streams and restart the combined conversion stream.
+		function onError( err ) {
+			if ( !options.watch ) {
+				// To stop the whole execution ASAP.
+				process.exit( 1 );
+
+				return;
+			}
+
+			unpipeStreams();
+
+			gutil.log( 'Restarting...' );
+			startStreams();
+		}
+
+		function startStreams() {
+			inputStream = utils.noop();
+			conversionStream = createConversionStream();
+
+			codeStream
+				.pipe( inputStream )
+				.pipe( conversionStream )
+				.pipe( outputStream );
+		}
+
+		function unpipeStreams() {
+			codeStream.unpipe( inputStream );
+			conversionStream.unpipe( outputStream );
+		}
 	} );
 
 	return tasks;

+ 24 - 20
dev/tasks/gulp/utils.js

@@ -51,33 +51,37 @@ const utils = {
 		}
 
 		return babel( {
-			plugins: [
-				// Note: When plugin is specified by its name, Babel loads it from a context of a
-				// currently transpiled file (in our case - e.g. from ckeditor5-core/src/foo.js).
-				// Obviously that fails, since we have all the plugins installed only in ckeditor5/
-				// and we want to have them only there to avoid installing them dozens of times.
-				//
-				// Anyway, I haven't found in the docs that you can also pass a plugin instance here,
-				// but it works... so let's hope it will.
-				require( `babel-plugin-transform-es2015-modules-${ babelModuleTranspiler }` )
-			],
-			// Ensure that all paths ends with '.js' because Require.JS (unlike Common.JS/System.JS)
-			// will not add it to module names which look like paths.
-			resolveModuleSource: ( source ) => {
-				return utils.appendModuleExtension( source );
-			}
-		} );
+				plugins: [
+					// Note: When plugin is specified by its name, Babel loads it from a context of a
+					// currently transpiled file (in our case - e.g. from ckeditor5-core/src/foo.js).
+					// Obviously that fails, since we have all the plugins installed only in ckeditor5/
+					// and we want to have them only there to avoid installing them dozens of times.
+					//
+					// Anyway, I haven't found in the docs that you can also pass a plugin instance here,
+					// but it works... so let's hope it will.
+					require( `babel-plugin-transform-es2015-modules-${ babelModuleTranspiler }` )
+				],
+				// Ensure that all paths ends with '.js' because Require.JS (unlike Common.JS/System.JS)
+				// will not add it to module names which look like paths.
+				resolveModuleSource: ( source ) => {
+					return utils.appendModuleExtension( source );
+				}
+			} )
+			.on( 'error', function( err ) {
+				gutil.log( gutil.colors.red( `Error (Babel:${ format })` ) );
+				gutil.log( gutil.colors.red( err.message ) );
+				console.log( '\n' + err.codeFrame + '\n' );
+			} );
 	},
 
 	/**
-	 * Creates a function adding transpilation pipes to the `pipes` param.
-	 * Used to generate `formats.reduce()` callback where `formats` is an array
-	 * of formats that should be generated.
+	 * Creates a function generating convertion streams.
+	 * Used to generate `formats.reduce()` callback where `formats` is an array of formats that should be generated.
 	 *
 	 * @param {String} distDir The `dist/` directory path.
 	 * @returns {Function}
 	 */
-	addFormat( distDir ) {
+	getConversionStreamGenerator( distDir ) {
 		return ( pipes, format ) => {
 			const conversionPipes = [];
 

+ 1 - 1
src/path.js

@@ -87,4 +87,4 @@ function getBasePath() {
 	return path;
 }
 
-export default path;
+export default path;