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

Merge pull request #54 from ckeditor/t/53

Resolve symlinks before passing paths to the watcher.
Piotr Jasiun 10 лет назад
Родитель
Сommit
f5886ef151
4 измененных файлов с 177 добавлено и 57 удалено
  1. 131 16
      dev/tasks/gulp/build.js
  2. 45 39
      dev/tasks/gulp/utils.js
  3. 0 1
      package.json
  4. 1 1
      src/path.js

+ 131 - 16
dev/tasks/gulp/build.js

@@ -19,16 +19,17 @@ const KNOWN_OPTIONS = {
 	}
 };
 
+const fs = require( 'fs' );
 const path = require( 'path' );
 const gulp = require( 'gulp' );
 const del = require( 'del' );
 const merge = require( 'merge-stream' );
 const gulpMirror = require( 'gulp-mirror' );
+const gulpWatch = require( 'gulp-watch' );
 const gutil = require( 'gulp-util' );
 const minimist = require( 'minimist' );
 const utils = require( './utils' );
 
-const sep = path.sep;
 const options = minimist( process.argv.slice( 2 ), KNOWN_OPTIONS[ process.argv[ 2 ] ] );
 
 module.exports = ( config ) => {
@@ -56,37 +57,74 @@ module.exports = ( config ) => {
 			/**
 			 * Returns a stream with just the main file (`ckeditor5/ckeditor.js`).
 			 *
-			 * @param {Boolean} [watch] Whether the files should be watched.
+			 * @param {Boolean} [watch] Whether to watch the files.
 			 * @returns {Stream}
 			 */
 			main( watch ) {
-				return utils.src( config.ROOT_DIR, 'ckeditor.js', watch );
+				const glob = path.join( config.ROOT_DIR, 'ckeditor.js' );
+
+				return gulp.src( glob )
+					.pipe( watch ? gulpWatch( glob ) : utils.noop() );
 			},
 
 			/**
 			 * Returns a stream of all source files from CKEditor 5.
 			 *
-			 * @param {Boolean} [watch] Whether the files should be watched.
+			 * @param {Boolean} [watch] Whether to watch the files.
 			 * @returns {Stream}
 			 */
 			ckeditor5( watch ) {
-				return utils.src( config.ROOT_DIR, 'src/**/*.js', watch )
+				const glob = path.join( config.ROOT_DIR, 'src', '**', '*.js' );
+
+				return gulp.src( glob )
+					.pipe( watch ? gulpWatch( glob ) : utils.noop() )
 					.pipe( utils.wrapCKEditor5Module() );
 			},
 
 			/**
 			 * Returns a stream of all source files from CKEditor 5 dependencies.
 			 *
-			 * @param {Boolean} [watch] Whether the files should be watched.
+			 * @param {Boolean} [watch] Whether to watch the files.
 			 * @returns {Stream}
 			 */
 			modules( watch ) {
-				// For an odd reason file.dirname does not contain `node_modules/`. Maybe the base dir
-				// is automatically set to only the varying piece of the path.
-				const modulePathPattern = new RegExp( `(ckeditor5-[^${ sep }]+)${ sep }src` );
-
-				return utils.src( config.ROOT_DIR, 'node_modules/ckeditor5-*/src/**/*.js', watch )
-					.pipe( utils.unpackModules( modulePathPattern ) );
+				// Find all CKEditor5 package directories. Resolve symlinks so we watch real directories
+				// in order to workaround https://github.com/paulmillr/chokidar/issues/419.
+				const dirs = fs.readdirSync( path.join( config.ROOT_DIR, 'node_modules' ) )
+					// Look for ckeditor5-* directories.
+					.filter( ( fileName ) => fileName.indexOf( 'ckeditor5-' ) === 0 )
+					// Resolve symlinks and keep only directories.
+					.map( ( fileName ) => {
+						let filePath = path.join( config.ROOT_DIR, 'node_modules', fileName );
+						let stat = fs.lstatSync( filePath );
+
+						if ( stat.isSymbolicLink() ) {
+							filePath = fs.realpathSync( filePath );
+							stat = fs.lstatSync( filePath );
+						}
+
+						if ( stat.isDirectory() ) {
+							return filePath;
+						}
+
+						// Filter...
+						return false;
+					} )
+					// 					...those out.
+					.filter( ( filePath ) => filePath );
+
+				const streams = dirs.map( ( dirPath ) => {
+					const glob = path.join( dirPath, 'src', '**', '*.js' );
+					// Use parent as a base so we get paths starting with 'ckeditor5-*/src/*' in the stream.
+					const baseDir = path.parse( dirPath ).dir;
+					const opts = { base: baseDir };
+
+					return gulp.src( glob, opts )
+						.pipe( watch ? gulpWatch( glob, opts ) : utils.noop() );
+				} );
+
+				return merge.apply( null, streams )
+					.pipe( utils.unpackModules() );
 			}
 		}
 	};
@@ -94,16 +132,93 @@ 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 );
+
+		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 );
+		}
 
-		return codeStream
-			.pipe( gulpMirror.apply( null, formatPipes ) );
+		// Handles error in the combined conversion stream.
+		// If we don't watch files, make sure that the process terminates ASAP. We could forward the error
+		// to the output, but there may be some data in the pipeline and our error could be covered
+		// by dozen of other messages.
+		// If we watch files, then clean up the old streams and restart the combined conversion stream.
+		function onError() {
+			if ( !options.watch ) {
+				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;
-};
+};

+ 45 - 39
dev/tasks/gulp/utils.js

@@ -6,34 +6,18 @@ const path = require( 'path' );
 const gulp = require( 'gulp' );
 const rename = require( 'gulp-rename' );
 const babel = require( 'gulp-babel' );
-const gulpWatch = require( 'gulp-watch' );
-const gulpPlumber = require( 'gulp-plumber' );
 const gutil = require( 'gulp-util' );
 const multipipe = require( 'multipipe' );
-
-const sep = path.sep;
+const PassThrough = require( 'stream' ).PassThrough;
 
 const utils = {
 	/**
-	 * Returns a stream of files matching the given glob pattern.
+	 * Creates a pass-through stream.
 	 *
-	 * @param {String} root The root directory.
-	 * @param {String} glob The glob pattern.
-	 * @param {Boolean} [watch] Whether to watch the files.
 	 * @returns {Stream}
 	 */
-	src( root, glob, watch ) {
-		const srcDir = path.join( root, glob );
-		let stream = gulp.src( srcDir );
-
-		if ( watch ) {
-			stream = stream
-				// Let's use plumber only when watching. In other cases we should fail quickly and loudly.
-				.pipe( gulpPlumber() )
-				.pipe( gulpWatch( srcDir ) );
-		}
-
-		return stream;
+	noop() {
+		return new PassThrough( { objectMode: true } );
 	},
 
 	/**
@@ -67,24 +51,37 @@ const utils = {
 		}
 
 		return babel( {
-			plugins: [ `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 = [];
 
@@ -125,19 +122,28 @@ const utils = {
 	},
 
 	/**
-	 * Moves files out of `node_modules/ckeditor5-xxx/src/*` directories to `ckeditor5-xxx/*`.
+	 * Moves files out of `ckeditor5-xxx/src/*` directories to `ckeditor5-xxx/*`.
 	 *
-	 * @param {RegExp} modulePathPattern
 	 * @returns {Stream}
 	 */
-	unpackModules( modulePathPattern ) {
+	unpackModules() {
 		return rename( ( file ) => {
-			file.dirname = file.dirname.replace( modulePathPattern, `${ sep }$1${ sep }` );
+			const dir = file.dirname.split( path.sep );
 
-			// Remove now empty src/ dirs.
-			if ( !file.extname && file.basename == 'src' ) {
-				file.basename = '';
+			// Validate the input for the clear conscious.
+
+			if ( dir[ 0 ].indexOf( 'ckeditor5-' ) !== 0 ) {
+				throw new Error( 'Path should start with "ckeditor5-".' );
+			}
+
+			if ( dir[ 1 ] != 'src' ) {
+				throw new Error( 'Path should start with "ckeditor5-*/src".' );
 			}
+
+			// Remove 'src'.
+			dir.splice( 1, 1 );
+
+			file.dirname = path.join.apply( null, dir );
 		} );
 	},
 
@@ -167,4 +173,4 @@ const utils = {
 	}
 };
 
-module.exports = utils;
+module.exports = utils;

+ 0 - 1
package.json

@@ -31,7 +31,6 @@
     "gulp": "^3.9.0",
     "gulp-babel": "^6.1.0",
     "gulp-mirror": "^0.4.0",
-    "gulp-plumber": "^1.0.1",
     "gulp-rename": "^1.2.2",
     "gulp-sourcemaps": "^1.6.0",
     "gulp-util": "^3.0.7",

+ 1 - 1
src/path.js

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