Convert use_require.js to use promises

We had some race conditions between the callbacks that could cause
failures. Order everything properly using promises.

diff --git a/utils/use_require.js b/utils/use_require.js
index 050efa252e1cfbb9e20e883f75664be6babb2c25..2e1e2d07e73eb7189cb6da223aa4545483534a35 100755 (executable)
--- a/utils/use_require.js
+++ b/utils/use_require.js
@@ -4,6 +4,7 @@ var path = require('path');
 var program = require('commander');
 var fs = require('fs');
 var fse = require('fs-extra');
+var babel = require('babel-core');
 
 const SUPPORTED_FORMATS = new Set(['amd', 'commonjs', 'systemjs', 'umd']);
 
@@ -38,23 +39,47 @@ const no_transform_files = new Set([
 
 no_copy_files.forEach((file) => no_transform_files.add(file));
 
+// util.promisify requires Node.js 8.x, so we have our own
+function promisify(original) {
+    return function () {
+        let obj = this;
+        let args = Array.prototype.slice.call(arguments);
+        return new Promise((resolve, reject) => {
+            original.apply(obj, args.concat((err, value) => {
+                if (err) return reject(err);
+                resolve(value);
+            }));
+        });
+    }
+}
+
+const readFile = promisify(fs.readFile);
+const writeFile = promisify(fs.writeFile);
+
+const readdir = promisify(fs.readdir);
+const lstat = promisify(fs.lstat);
+
+const copy = promisify(fse.copy);
+const ensureDir = promisify(fse.ensureDir);
+
+const babelTransformFile = promisify(babel.transformFile);
+
 // walkDir *recursively* walks directories trees,
 // calling the callback for all normal files found.
 var walkDir = function (base_path, cb, filter) {
-    fs.readdir(base_path, (err, files) => {
-        if (err) throw err;
-
-        files.map((filename) => path.join(base_path, filename)).forEach((filepath) => {
-            fs.lstat(filepath, (err, stats) => {
-                if (err) throw err;
-
+    return readdir(base_path)
+    .then(files => {
+        let paths = files.map(filename => path.join(base_path, filename));
+        return Promise.all(paths.map((filepath) => {
+            return lstat(filepath)
+            .then(stats => {
                 if (filter !== undefined && !filter(filepath, stats)) return;
 
                 if (stats.isSymbolicLink()) return;
-                if (stats.isFile()) cb(filepath);
-                if (stats.isDirectory()) walkDir(filepath, cb, filter);
+                if (stats.isFile()) return cb(filepath);
+                if (stats.isDirectory()) return walkDir(filepath, cb, filter);
             });
-        });
+        }));
     });
 };
 
@@ -62,9 +87,8 @@ var transform_html = function (new_script) {
     // write out the modified vnc.html file that works with the bundle
     var src_html_path = path.resolve(__dirname, '..', 'vnc.html');
     var out_html_path = path.resolve(paths.out_dir_base, 'vnc.html');
-    fs.readFile(src_html_path, (err, contents_raw) => {
-        if (err) { throw err; }
-
+    return readFile(src_html_path)
+    .then(contents_raw => {
         var contents = contents_raw.toString();
 
         var start_marker = '<!-- begin scripts -->\n';
@@ -74,10 +98,11 @@ var transform_html = function (new_script) {
 
         contents = contents.slice(0, start_ind) + `${new_script}\n` + contents.slice(end_ind);
 
+        return contents;
+    })
+    .then((contents) => {
         console.log(`Writing ${out_html_path}`);
-        fs.writeFile(out_html_path, contents, function (err) {
-            if (err) { throw err; }
-        });
+        return writeFile(out_html_path, contents);
     });
 }
 
@@ -94,7 +119,6 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) {
         ast: false,
         sourceMaps: source_maps,
     });
-    const babel = require('babel-core');
 
     var in_path;
     if (with_app_dir) {
@@ -109,23 +133,24 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) {
     const helpers = require('./use_require_helpers');
     const helper = helpers[import_format];
 
-    var handleDir = (js_only, vendor_rewrite, in_path_base, filename) => {
+    var handleDir = (js_only, vendor_rewrite, in_path_base, filename) => Promise.resolve()
+    .then(() => {
         if (no_copy_files.has(filename)) return;
 
         const out_path = path.join(out_path_base, path.relative(in_path_base, filename));
         if(path.extname(filename) !== '.js') {
             if (!js_only) {
                 console.log(`Writing ${out_path}`);
-                fse.copy(filename, out_path, (err) => { if (err) throw err; });
+                return copy(filename, out_path);
             }
             return;  // skip non-javascript files
         }
 
-        fse.ensureDir(path.dirname(out_path), () => {
+        return ensureDir(path.dirname(out_path))
+        .then(() => {
             if (no_transform_files.has(filename)) {
                 console.log(`Writing ${out_path}`);
-                fse.copy(filename, out_path, (err) => { if (err) throw err; });
-                return;
+                return copy(filename, out_path);
             }
 
             const opts = babel_opts();
@@ -140,42 +165,62 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) {
                                     "redirect": { "vendor/(.+)": "./vendor/$1"}}]);
             }
 
-            babel.transformFile(filename, opts, (err, res) => {
+            return babelTransformFile(filename, opts)
+            .then(res => {
                 console.log(`Writing ${out_path}`);
-                if (err) throw err;
                 var {code, map, ast} = res;
                 if (source_maps === true) {
                     // append URL for external source map
                     code += `\n//# sourceMappingURL=${path.basename(out_path)}.map\n`;
                 }
-                fs.writeFile(out_path, code, (err) => { if (err) throw err; });
-                if (source_maps === true || source_maps === 'both') {
-                    console.log(`  and ${out_path}.map`);
-                    fs.writeFile(`${out_path}.map`, JSON.stringify(map), (err) => { if (err) throw err; });
-                }
+                return writeFile(out_path, code)
+                .then(() => {
+                    if (source_maps === true || source_maps === 'both') {
+                        console.log(`  and ${out_path}.map`);
+                        return writeFile(`${out_path}.map`, JSON.stringify(map));
+                    }
+                });
             });
         });
-    };
+    });
 
     if (with_app_dir && helper && helper.noCopyOverride) {
         helper.noCopyOverride(paths, no_copy_files);
     }
 
-    walkDir(paths.vendor, handleDir.bind(null, true, false, in_path || paths.main), (filename, stats) => !no_copy_files.has(filename));
-    walkDir(paths.core, handleDir.bind(null, true, !in_path, in_path || paths.core), (filename, stats) => !no_copy_files.has(filename));
-
-    if (with_app_dir) {
-        walkDir(paths.app, handleDir.bind(null, false, false, in_path), (filename, stats) => !no_copy_files.has(filename));
+    Promise.resolve()
+    .then(() => {
+        let handler = handleDir.bind(null, true, false, in_path || paths.main);
+        let filter = (filename, stats) => !no_copy_files.has(filename);
+        return walkDir(paths.vendor, handler, filter);
+    })
+    .then(() => {
+        let handler = handleDir.bind(null, true, !in_path, in_path || paths.core);
+        let filter = (filename, stats) => !no_copy_files.has(filename);
+        return walkDir(paths.core, handler, filter);
+    })
+    .then(() => {
+        if (!with_app_dir) return;
+        let handler = handleDir.bind(null, false, false, in_path);
+        let filter = (filename, stats) => !no_copy_files.has(filename);
+        return walkDir(paths.app, handler, filter);
+    })
+    .then(() => {
+        if (!with_app_dir) return;
+
+        if (!helper || !helper.appWriter) {
+            throw new Error(`Unable to generate app for the ${import_format} format!`);
+        }
 
         const out_app_path = path.join(out_path_base, 'app.js');
-        if (helper && helper.appWriter) {
-            console.log(`Writing ${out_app_path}`);
-            let out_script = helper.appWriter(out_path_base, out_app_path);
-            transform_html(out_script);
-        } else {
-            console.error(`Unable to generate app for the ${import_format} format!`);
-        }
-    }
+        console.log(`Writing ${out_app_path}`);
+        return helper.appWriter(out_path_base, out_app_path)
+        .then(transform_html);
+    })
+    .catch((err) => {
+        console.error(`Failure converting modules: ${err}`);
+        process.exit(1);
+    });
 };
 
 if (program.clean) {

diff --git a/utils/use_require_helpers.js b/utils/use_require_helpers.js
index 990fb4df2481331e5998c7b91c39a377bba3dd40..66598543025526b35c1bc1b43f9f716f8f254775 100644 (file)
--- a/utils/use_require_helpers.js
+++ b/utils/use_require_helpers.js
@@ -3,13 +3,31 @@ var fs = require('fs');
 var fse = require('fs-extra');
 var path = require('path');
 
+// util.promisify requires Node.js 8.x, so we have our own
+function promisify(original) {
+    return function () {
+        let obj = this;
+        let args = Array.prototype.slice.call(arguments);
+        return new Promise((resolve, reject) => {
+            original.apply(obj, args.concat((err, value) => {
+                if (err) return reject(err);
+                resolve(value);
+            }));
+        });
+    }
+}
+
+const writeFile = promisify(fs.writeFile);
+
 module.exports = {
     'amd': {
         appWriter: (base_out_path, out_path) => {
             // setup for requirejs
-            fs.writeFile(out_path, 'requirejs(["app/ui"], function (ui) {});', (err) => { if (err) throw err; });
-            console.log(`Please place RequireJS in ${path.join(base_out_path, 'require.js')}`);
-            return `<script src="require.js" data-main="${path.relative(base_out_path, out_path)}"></script>`;
+            return writeFile(out_path, 'requirejs(["app/ui"], function (ui) {});')
+            .then(() => {
+                console.log(`Please place RequireJS in ${path.join(base_out_path, 'require.js')}`);
+                return `<script src="require.js" data-main="${path.relative(base_out_path, out_path)}"></script>`;
+            });
         },
         noCopyOverride: () => {},
     },
@@ -21,17 +39,20 @@ module.exports = {
         appWriter: (base_out_path, out_path) => {
             var browserify = require('browserify');
             var b = browserify(path.join(base_out_path, 'app/ui.js'), {});
-            b.bundle().pipe(fs.createWriteStream(out_path));
-            return `<script src="${path.relative(base_out_path, out_path)}"></script>`;
+            return promisify(b.bundle).call(b)
+            .then((buf) => writeFile(out_path, buf))
+            .then(() => `<script src="${path.relative(base_out_path, out_path)}"></script>`);
         },
         noCopyOverride: () => {},
     },
     'systemjs': {
         appWriter: (base_out_path, out_path) => {
-            fs.writeFile(out_path, 'SystemJS.import("./app/ui.js");', (err) => { if (err) throw err; });
-            console.log(`Please place SystemJS in ${path.join(base_out_path, 'system-production.js')}`);
-            return `<script src="vendor/promise.js"></script>
+            return writeFile(out_path, 'SystemJS.import("./app/ui.js");')
+            .then(() => {
+                console.log(`Please place SystemJS in ${path.join(base_out_path, 'system-production.js')}`);
+                return `<script src="vendor/promise.js"></script>
 <script src="system-production.js"></script>\n<script src="${path.relative(base_out_path, out_path)}"></script>`;
+            });
         },
         noCopyOverride: (paths, no_copy_files) => {
             no_copy_files.delete(path.join(paths.vendor, 'promise.js'));