From 2163326888129b34f9dc2b836b03985bc4fab0e2 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sat, 9 Dec 2017 13:46:21 +0100 Subject: [PATCH 1/4] Convert use_require.js to use promises We had some race conditions between the callbacks that could cause failures. Order everything properly using promises. --- utils/use_require.js | 129 +++++++++++++++++++++++------------ utils/use_require_helpers.js | 37 +++++++--- 2 files changed, 116 insertions(+), 50 deletions(-) diff --git a/utils/use_require.js b/utils/use_require.js index 050efa25..2e1e2d07 100755 --- 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 = '\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)); + 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 (with_app_dir) { - walkDir(paths.app, handleDir.bind(null, false, false, in_path), (filename, stats) => !no_copy_files.has(filename)); + 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 990fb4df..66598543 100644 --- 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 ``; + return writeFile(out_path, 'requirejs(["app/ui"], function (ui) {});') + .then(() => { + console.log(`Please place RequireJS in ${path.join(base_out_path, 'require.js')}`); + return ``; + }); }, 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 ``; + return promisify(b.bundle).call(b) + .then((buf) => writeFile(out_path, buf)) + .then(() => ``); }, 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 ` + 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 ` \n`; + }); }, noCopyOverride: (paths, no_copy_files) => { no_copy_files.delete(path.join(paths.vendor, 'promise.js')); From be7b4e88f003cea6f90b67a3d347b30272fb7512 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sat, 9 Dec 2017 14:14:04 +0100 Subject: [PATCH 2/4] Remove intermediate files when bundling --- utils/use_require.js | 27 ++++++++++++++++++++++++++- utils/use_require_helpers.js | 1 + 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/utils/use_require.js b/utils/use_require.js index 2e1e2d07..cfac079c 100755 --- a/utils/use_require.js +++ b/utils/use_require.js @@ -60,7 +60,9 @@ const readdir = promisify(fs.readdir); const lstat = promisify(fs.lstat); const copy = promisify(fse.copy); +const unlink = promisify(fse.unlink); const ensureDir = promisify(fse.ensureDir); +const rmdir = promisify(fse.rmdir); const babelTransformFile = promisify(babel.transformFile); @@ -133,6 +135,8 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { const helpers = require('./use_require_helpers'); const helper = helpers[import_format]; + const outFiles = []; + var handleDir = (js_only, vendor_rewrite, in_path_base, filename) => Promise.resolve() .then(() => { if (no_copy_files.has(filename)) return; @@ -173,10 +177,12 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { // append URL for external source map code += `\n//# sourceMappingURL=${path.basename(out_path)}.map\n`; } + outFiles.push(`${out_path}`); return writeFile(out_path, code) .then(() => { if (source_maps === true || source_maps === 'both') { console.log(` and ${out_path}.map`); + outFiles.push(`${out_path}.map`); return writeFile(`${out_path}.map`, JSON.stringify(map)); } }); @@ -215,7 +221,26 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { const out_app_path = path.join(out_path_base, 'app.js'); console.log(`Writing ${out_app_path}`); return helper.appWriter(out_path_base, out_app_path) - .then(transform_html); + .then(transform_html) + .then(() => { + if (!helper.removeModules) return; + console.log(`Cleaning up temporary files...`); + return Promise.all(outFiles.map(filepath => { + unlink(filepath) + .then(() => { + // Try to clean up any empty directories if this + // was the last file in there + let rmdir_r = dir => { + return rmdir(dir) + .then(() => rmdir_r(path.dirname(dir))) + .catch(() => { + // Assume the error was ENOTEMPTY and ignore it + }); + }; + return rmdir_r(path.dirname(filepath)); + }); + })); + }); }) .catch((err) => { console.error(`Failure converting modules: ${err}`); diff --git a/utils/use_require_helpers.js b/utils/use_require_helpers.js index 66598543..d9138ba4 100644 --- a/utils/use_require_helpers.js +++ b/utils/use_require_helpers.js @@ -44,6 +44,7 @@ module.exports = { .then(() => ``); }, noCopyOverride: () => {}, + removeModules: true, }, 'systemjs': { appWriter: (base_out_path, out_path) => { From d584c5f624118e7eaf2bf5e5df291566150e7154 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sat, 9 Dec 2017 14:14:23 +0100 Subject: [PATCH 3/4] Don't include icons Makefile when packaging --- utils/use_require.js | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/use_require.js b/utils/use_require.js index cfac079c..d126580e 100755 --- a/utils/use_require.js +++ b/utils/use_require.js @@ -30,6 +30,7 @@ const no_copy_files = new Set([ path.join(paths.vendor, 'sinon.js'), path.join(paths.vendor, 'browser-es-module-loader'), path.join(paths.vendor, 'promise.js'), + path.join(paths.app, 'images', 'icons', 'Makefile'), ]); const no_transform_files = new Set([ From 4a65d50d0c57e76310ab965a57032172596a60bf Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Mon, 11 Dec 2017 16:35:40 +0100 Subject: [PATCH 4/4] Only use converted modules as legacy fallback for app Several of the major browsers now natively support modules, so we only need the converted modules to handle older browsers. Make sure it's only used when necessary. --- utils/use_require.js | 86 +++++++++++++++++++++++++++++------- utils/use_require_helpers.js | 34 +++++++++----- 2 files changed, 92 insertions(+), 28 deletions(-) diff --git a/utils/use_require.js b/utils/use_require.js index d126580e..876f6525 100755 --- a/utils/use_require.js +++ b/utils/use_require.js @@ -12,6 +12,7 @@ program .option('--as [format]', `output files using various import formats instead of ES6 import and export. Supports ${Array.from(SUPPORTED_FORMATS)}.`) .option('-m, --with-source-maps [type]', 'output source maps when not generating a bundled app (type may be empty for external source maps, inline for inline source maps, or both) ') .option('--with-app', 'process app files as well as core files') + .option('--only-legacy', 'only output legacy files (no ES6 modules) for the app') .option('--clean', 'clear the lib folder before building') .parse(process.argv); @@ -86,7 +87,7 @@ var walkDir = function (base_path, cb, filter) { }); }; -var transform_html = function (new_script) { +var transform_html = function (legacy_scripts, only_legacy) { // 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'); @@ -99,6 +100,37 @@ var transform_html = function (new_script) { var start_ind = contents.indexOf(start_marker) + start_marker.length; var end_ind = contents.indexOf(end_marker, start_ind); + new_script = ''; + + if (only_legacy) { + // Only legacy version, so include things directly + for (let i = 0;i < legacy_scripts.length;i++) { + new_script += ` \n`; + } + } else { + // Otherwise detect if it's a modern browser and select + // variant accordingly + new_script += `\ + \n\ + \n`; + + // Original, ES6 modules + new_script += ' \n'; + } + contents = contents.slice(0, start_ind) + `${new_script}\n` + contents.slice(end_ind); return contents; @@ -109,7 +141,7 @@ var transform_html = function (new_script) { }); } -var make_lib_files = function (import_format, source_maps, with_app_dir) { +var make_lib_files = function (import_format, source_maps, with_app_dir, only_legacy) { if (!import_format) { throw new Error("you must specify an import format to generate compiled noVNC libraries"); } else if (!SUPPORTED_FORMATS.has(import_format)) { @@ -123,6 +155,11 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { sourceMaps: source_maps, }); + // No point in duplicate files without the app, so force only converted files + if (!with_app_dir) { + only_legacy = true; + } + var in_path; if (with_app_dir) { var out_path_base = paths.out_dir_base; @@ -130,6 +167,7 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { } else { var out_path_base = paths.lib_dir_base; } + const legacy_path_base = only_legacy ? out_path_base : path.join(out_path_base, 'legacy'); fse.ensureDirSync(out_path_base); @@ -143,6 +181,8 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { if (no_copy_files.has(filename)) return; const out_path = path.join(out_path_base, path.relative(in_path_base, filename)); + const legacy_path = path.join(legacy_path_base, path.relative(in_path_base, filename)); + if(path.extname(filename) !== '.js') { if (!js_only) { console.log(`Writing ${out_path}`); @@ -151,11 +191,21 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { return; // skip non-javascript files } - return ensureDir(path.dirname(out_path)) + return Promise.resolve() .then(() => { - if (no_transform_files.has(filename)) { + if (only_legacy && !no_transform_files.has(filename)) { + return; + } + return ensureDir(path.dirname(out_path)) + .then(() => { console.log(`Writing ${out_path}`); return copy(filename, out_path); + }) + }) + .then(() => ensureDir(path.dirname(legacy_path))) + .then(() => { + if (no_transform_files.has(filename)) { + return; } const opts = babel_opts(); @@ -166,25 +216,25 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { // to the vendor directory if (vendor_rewrite) { opts.plugins.push(["import-redirect", - {"root": out_path_base, + {"root": legacy_path_base, "redirect": { "vendor/(.+)": "./vendor/$1"}}]); } return babelTransformFile(filename, opts) .then(res => { - console.log(`Writing ${out_path}`); + console.log(`Writing ${legacy_path}`); var {code, map, ast} = res; if (source_maps === true) { // append URL for external source map - code += `\n//# sourceMappingURL=${path.basename(out_path)}.map\n`; + code += `\n//# sourceMappingURL=${path.basename(legacy_path)}.map\n`; } - outFiles.push(`${out_path}`); - return writeFile(out_path, code) + outFiles.push(`${legacy_path}`); + return writeFile(legacy_path, code) .then(() => { if (source_maps === true || source_maps === 'both') { - console.log(` and ${out_path}.map`); - outFiles.push(`${out_path}.map`); - return writeFile(`${out_path}.map`, JSON.stringify(map)); + console.log(` and ${legacy_path}.map`); + outFiles.push(`${legacy_path}.map`); + return writeFile(`${legacy_path}.map`, JSON.stringify(map)); } }); }); @@ -219,10 +269,14 @@ var make_lib_files = function (import_format, source_maps, with_app_dir) { throw new Error(`Unable to generate app for the ${import_format} format!`); } - const out_app_path = path.join(out_path_base, 'app.js'); + const out_app_path = path.join(legacy_path_base, 'app.js'); console.log(`Writing ${out_app_path}`); - return helper.appWriter(out_path_base, out_app_path) - .then(transform_html) + return helper.appWriter(out_path_base, legacy_path_base, out_app_path) + .then(extra_scripts => { + let rel_app_path = path.relative(out_path_base, out_app_path); + let legacy_scripts = extra_scripts.concat([rel_app_path]); + transform_html(legacy_scripts, only_legacy); + }) .then(() => { if (!helper.removeModules) return; console.log(`Cleaning up temporary files...`); @@ -257,4 +311,4 @@ if (program.clean) { fse.removeSync(paths.out_dir_base); } -make_lib_files(program.as, program.withSourceMaps, program.withApp); +make_lib_files(program.as, program.withSourceMaps, program.withApp, program.onlyLegacy); diff --git a/utils/use_require_helpers.js b/utils/use_require_helpers.js index d9138ba4..19a43b77 100644 --- a/utils/use_require_helpers.js +++ b/utils/use_require_helpers.js @@ -21,12 +21,16 @@ const writeFile = promisify(fs.writeFile); module.exports = { 'amd': { - appWriter: (base_out_path, out_path) => { + appWriter: (base_out_path, script_base_path, out_path) => { // setup for requirejs - return writeFile(out_path, 'requirejs(["app/ui"], function (ui) {});') + let ui_path = path.relative(base_out_path, + path.join(script_base_path, 'app', 'ui')); + return writeFile(out_path, `requirejs(["${ui_path}"], function (ui) {});`) .then(() => { - console.log(`Please place RequireJS in ${path.join(base_out_path, 'require.js')}`); - return ``; + console.log(`Please place RequireJS in ${path.join(script_base_path, 'require.js')}`); + let require_path = path.relative(base_out_path, + path.join(script_base_path, 'require.js')) + return [ require_path ]; }); }, noCopyOverride: () => {}, @@ -36,23 +40,29 @@ module.exports = { // CommonJS supports properly shifting the default export to work as normal opts.plugins.unshift("add-module-exports"); }, - appWriter: (base_out_path, out_path) => { + appWriter: (base_out_path, script_base_path, out_path) => { var browserify = require('browserify'); - var b = browserify(path.join(base_out_path, 'app/ui.js'), {}); + var b = browserify(path.join(script_base_path, 'app/ui.js'), {}); return promisify(b.bundle).call(b) .then((buf) => writeFile(out_path, buf)) - .then(() => ``); + .then(() => []); }, noCopyOverride: () => {}, removeModules: true, }, 'systemjs': { - appWriter: (base_out_path, out_path) => { - return writeFile(out_path, 'SystemJS.import("./app/ui.js");') + appWriter: (base_out_path, script_base_path, out_path) => { + let ui_path = path.relative(base_out_path, + path.join(script_base_path, 'app', 'ui.js')); + return writeFile(out_path, `SystemJS.import("${ui_path}");`) .then(() => { - console.log(`Please place SystemJS in ${path.join(base_out_path, 'system-production.js')}`); - return ` -\n`; + console.log(`Please place SystemJS in ${path.join(script_base_path, 'system-production.js')}`); + // FIXME: Should probably be in the legacy directory + let promise_path = path.relative(base_out_path, + path.join(base_out_path, 'vendor', 'promise.js')) + let systemjs_path = path.relative(base_out_path, + path.join(script_base_path, 'system-production.js')) + return [ promise_path, systemjs_path ]; }); }, noCopyOverride: (paths, no_copy_files) => {