I've got a gulp task that loops through a folder looking for sub folders and outputs a JavaScript file based upon the contents of each folder. Below is a more visual example.
becomes:
This is achieved by reading the contents of the src/assets/scripts directory, then running a loop against each folder (critical, legacy, modern, service-worker) and sending the contents of each folder to a Gulp tasks which get merged together with merge-stream.
All this works great, except that once the tasks are merged back together, I want to trigger a notification if the compilation succeeded. If I try to pipe anything to the merged streams, it doesn't work. It just returns the merged streams, and never continues on.
If I un-promisify my PROCESS_SCRIPTS function and don't use merge-stream (i.e. only processing one manually specified folder), it works fine, so I'm at a loss as to what's going on.
Here's my full task:
module.exports = {
scripts(gulp, plugins, ran_tasks, on_error) {
// task-specific plugins
const ESLINT = require("gulp-eslint");
const WEBPACK = require("webpack-stream");
// process scripts
const PROCESS_SCRIPTS = (js_directory, destination_file_name = "modern.js", compare_file_name = "modern.js", source = [global.settings.paths.src + "/assets/scripts/*.js"]) => {
return new Promise((resolve, reject) => {
const WEBPACK_CONFIG = {
mode: "development",
};
// update webpack config for the current target destination and file name
WEBPACK_CONFIG.mode = plugins.argv.dist ? "production" : WEBPACK_CONFIG.mode;
WEBPACK_CONFIG.output = {
filename: destination_file_name
};
const TASK = gulp.src(source)
// prevent breaking on error
.pipe(plugins.plumber({errorHandler: on_error}))
// check if source is newer than destination
.pipe(plugins.newer(js_directory + "/" + compare_file_name))
// lint all scripts
.pipe(ESLINT())
// print lint errors
.pipe(ESLINT.format())
// run webpack
.pipe(WEBPACK(WEBPACK_CONFIG))
// generate a hash and add it to the file name
.pipe(plugins.hash({template: "<%= name %>.<%= hash %><%= ext %>"}))
// output scripts to compiled directory
.pipe(gulp.dest(js_directory))
// generate a hash manfiest
.pipe(plugins.hash.manifest(".hashmanifest-scripts", {
deleteOld: true,
sourceDir: js_directory
}))
// output hash manifest in root
.pipe(gulp.dest("."))
// reject after errors
.on("error", () => {
reject(TASK);
})
// return the task after completion
.on("end", () => {
resolve(TASK);
});
});
};
// scripts task, lints, concatenates, & compresses JS
return new Promise ((resolve) => {
// set JS directory
const JS_DIRECTORY = plugins.argv.dist ? global.settings.paths.dist + "/assets/scripts" : global.settings.paths.dev + "/assets/scripts";
// set the source directory
const SOURCE_DIRECTORY = global.settings.paths.src + "/assets/scripts";
// set up an empty merged stream
const MERGED_STREAMS = plugins.merge();
// get the script source folder list
const SCRIPT_FOLDERS = plugins.fs.readdirSync(SOURCE_DIRECTORY);
// get the script destination file list
const SCRIPT_FILES = plugins.fs.existsSync(JS_DIRECTORY) ? plugins.fs.readdirSync(JS_DIRECTORY) : false;
// process all the script folders
const PROCESS_SCRIPT_FOLDERS = () => {
return Promise.resolve().then(() => {
// shift to the next folder
const FOLDER_NAME = SCRIPT_FOLDERS.shift();
// find the existing destination script file name
const FILE_NAME = SCRIPT_FILES ? SCRIPT_FILES.find((name) => {
return name.match(new RegExp(FOLDER_NAME + ".[a-z0-9]{8}.js"));
}) : FOLDER_NAME + ".js";
// process all scripts, update the stream
return PROCESS_SCRIPTS(JS_DIRECTORY, FOLDER_NAME + ".js", FILE_NAME, SOURCE_DIRECTORY + "/" + FOLDER_NAME + "/**/*").then((processed) => {
MERGED_STREAMS.add(processed);
});
}).then(() => SCRIPT_FOLDERS.length > 0 ? PROCESS_SCRIPT_FOLDERS() : resolve());
};
PROCESS_SCRIPT_FOLDERS().then(() => {
// wrap up
return MERGED_STREAMS
// prevent breaking on error
.pipe(plugins.plumber({
errorHandler: on_error,
}))
// notify that task is complete, if not part of default or watch
.pipe(plugins.gulpif(gulp.seq.indexOf("scripts") > gulp.seq.indexOf("default"), plugins.notify({
title: "Success!",
message: "Scripts task complete!",
onLast: true,
})))
// push task to ran_tasks array
.on("data", () => {
if (ran_tasks.indexOf("scripts") < 0) {
ran_tasks.push("scripts");
}
})
// resolve the promise on end
.on("end", () => {
resolve();
});
});
});
}
};
Also visible on my GitHub: https://github.com/JacobDB/new-site/blob/master/gulp-tasks/scripts.js
EDIT: I've tried a few things, I'll detail them here.
console.log("hello world") never fires after MERGED_STREAMS.on("data"), MERGED_STREAMS.on("error"), or MERGED_STREAMS.on("end").const MERGED_STREAMS = plugins.merge(); to a module-level variable (i.e. just after const WEBPACK = require("webpack-stream")) does not change the outcome.MERGED_STREAMS.add(gulp.src(source) ...) instead of adding the stream after the promise completes does not change the outcome, except when leaving .pipe(gulp.dist(".")), which is required to output a .hashmanifest, and always marks the task as ran.webpack, hash, or eslint, in any combination does not make a difference.PROCESS_SCRIPTS from returning a promise to return the stream, then processing each folder as individual variables, then merging them manually does appear to correctly trigger the task as ran, but webpack can only be run once, so it only outputs one file – critical.hash.js. Note: I haven't tested this method in conjunction with disabling hash, which may be causing it to be marked as correctly ran if .hashmanifest is always being output.lint task is not a promise, which results in unexpected end of stream errors in the console.EDIT 2: Updated with a revised version of my task based on @Louis's advice.
There are many problems with the code above. One major issue that makes the code hard to follow and debug is that you use new Promise where you don't need it. Generally, if you have new Promise and the logic inside the promise executor will resolve or reject depending on the result of another promise, then you don't need to use new Promise.
Sometimes people have code like this:
function foo() {
const a = getValueSynchronously();
const b = getOtherValueSynchronously();
return doSomethingAsynchronous(a, b).then(x => x.someMethod());
}
Suppose that doSomethigAsynchronous returns a promise. The problem with the foo function above is that if getValueSynchronously and getOtherValueSynchronously fail, then foo will raise an exception, but if doSomethingAsynchronous fails, then it will reject a promise. So code that uses foo has to handle synchronous exceptions and asynchronous rejections if it wants to handle all possible failures. Sometimes people feel they can fix the issue by causing all failures to be promise rejections:
function foo() {
return new Promise((resolve, reject) => {
const a = getValueSynchronously();
const b = getOtherValueSynchronously();
doSomethingAsynchronous(a, b).then(x => x.someMethod()).then(resolve, reject);
});
}
In the code above, if getValueSynchronously or getOtherValueSynchronously fail, that's a promise rejection. But the problem with the code above is that it is easy to get it wrong. You can forget to call reject everywhere it is needed. (As a matter of fact, you do have this error in your code. You have nested promises whose rejection won't be propagated up. They are just lost, which means if an error occurs your code will just stop, without you knowing why.) Or you may be tempted to call `resolve way down in a nested function, which makes the logic hard to follow.
You can just as well do:
function foo() {
return Promise.resolve().then(() => {
const a = getValueSynchronously();
const b = getOtherValueSynchronously();
return doSomethingAsynchronous(a, b);
}).then(x => x.someMethod());
}
You can use Promise.resolve() to enter the promisified world (hmm... "the promised land?"). In the code above, you do not have to remember to call reject. If the code inside the callback to .then fails for any reason, you get a rejected promise.
I also noticed in a number of places you return a value from the executor function you pass to new Promise. Your code would behave exactly the same if you did not use return there. To illustrate, this code:
function foo() {
return new Promise((resolve, reject) => {
return doSomethingAsynchronous().then(resolve, reject);
});
}
behaves the exactly same way as this code:
function foo() {
return new Promise((resolve, reject) => {
doSomethingAsynchronous().then(resolve, reject);
});
}
The value returned from the executor is ignored. End of story. If you think the value you return from your executors are doing something, then that's incorrect.