From 9e7e157990c6596e0f1d344b60f588ac4d60193e Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 19 Feb 2021 02:40:41 +0300 Subject: [PATCH 1/5] Fix reporting of config errors Errors are swallowed while resolving. --- bin/svgo | 6 +++- lib/svgo-node.js | 20 ++++++++----- lib/svgo/coa.js | 31 ++++++--------------- test/coa/_index.js | 12 +++----- test/config/_index.js | 11 ++++++++ test/config/fixtures/invalid/svgo.config.js | 1 + 6 files changed, 42 insertions(+), 39 deletions(-) create mode 100644 test/config/fixtures/invalid/svgo.config.js diff --git a/bin/svgo b/bin/svgo index 14b21b8e4..0c2945204 100755 --- a/bin/svgo +++ b/bin/svgo @@ -1,6 +1,10 @@ #!/usr/bin/env node +const { red } = require('chalk'); const { program } = require('commander'); const makeProgram = require('../lib/svgo/coa'); makeProgram(program); -program.parseAsync(process.argv); +program.parseAsync(process.argv).catch(error => { + console.error(red(error.stack)); + process.exit(1); +}); diff --git a/lib/svgo-node.js b/lib/svgo-node.js index ae9935719..f374711aa 100644 --- a/lib/svgo-node.js +++ b/lib/svgo-node.js @@ -23,6 +23,14 @@ const importConfig = async configFile => { } }; +const getStats = async (file) => { + try { + return await fs.promises.stat(file); + } catch { + return null; + } +} + const loadConfig = async (configFile, cwd = process.cwd()) => { if (configFile != null) { if (path.isAbsolute(configFile)) { @@ -33,13 +41,11 @@ const loadConfig = async (configFile, cwd = process.cwd()) => { } let dir = cwd; while (true) { - try { - const file = path.join(dir, "svgo.config.js"); - const stats = await fs.promises.stat(file); - if (stats.isFile()) { - return await importConfig(file); - } - } catch {} + const file = path.join(dir, "svgo.config.js"); + const stats = await getStats(file); + if (stats && stats.isFile()) { + return await importConfig(file); + } const parent = path.dirname(dir); if (dir === parent) { return null; diff --git a/lib/svgo/coa.js b/lib/svgo/coa.js index de7e17a96..4163540ec 100644 --- a/lib/svgo/coa.js +++ b/lib/svgo/coa.js @@ -9,7 +9,6 @@ const pluginsMap = require('../../plugins/plugins.js'); const PKG = require('../../package.json'); const { encodeSVGDatauri, decodeSVGDatauri } = require('./tools.js'); const regSVGFile = /\.svg$/i; -const noop = () => {}; /** * Synchronously check if path is a directory. Tolerant to errors like ENOENT. @@ -97,18 +96,14 @@ async function action(args, opts) { if (typeof process == 'object' && process.versions && process.versions.node && PKG && PKG.engines.node) { var nodeVersion = String(PKG.engines.node).match(/\d*(\.\d+)*/)[0]; if (parseFloat(process.versions.node) < parseFloat(nodeVersion)) { - return printErrorAndExit(`Error: ${PKG.name} requires Node.js version ${nodeVersion} or higher.`); + throw Error(`${PKG.name} requires Node.js version ${nodeVersion} or higher.`); } } // --config - try { - const loadedConfig = await loadConfig(opts.config); - if (loadedConfig != null) { - config = loadedConfig; - } - } catch (error) { - return printErrorAndExit(error.message); + const loadedConfig = await loadConfig(opts.config); + if (loadedConfig != null) { + config = loadedConfig; } // --quiet @@ -166,7 +161,7 @@ async function action(args, opts) { // --folder if (opts.folder) { var ouputFolder = output && output[0] || opts.folder; - return optimizeFolder(config, opts.folder, ouputFolder).then(noop, printErrorAndExit); + await optimizeFolder(config, opts.folder, ouputFolder); } // --input @@ -183,8 +178,9 @@ async function action(args, opts) { }); // file } else { - return Promise.all(input.map((file, n) => optimizeFile(config, file, output[n]))) - .then(noop, printErrorAndExit); + await Promise.all( + input.map((file, n) => optimizeFile(config, file, output[n])) + ); } // --string @@ -390,15 +386,4 @@ function showAvailablePlugins() { console.log('Currently available plugins:\n' + list); } -/** - * Write an error and exit. - * @param {Error} error - * @return {Promise} a promise for running tests - */ -function printErrorAndExit(error) { - console.error(chalk.red(error)); - process.exit(1); - return Promise.reject(error); // for tests -} - module.exports.checkIsDir = checkIsDir; diff --git a/test/coa/_index.js b/test/coa/_index.js index 6d87fb35f..8d2c01bc2 100644 --- a/test/coa/_index.js +++ b/test/coa/_index.js @@ -175,22 +175,18 @@ describe('coa', function() { if (!fs.existsSync(emptyFolderPath)) { fs.mkdirSync(emptyFolderPath); } - replaceConsoleError(); try { await runProgram(['--folder', emptyFolderPath, '--quiet']); - } catch {} finally { - restoreConsoleError(); - expect(output).to.match(/No SVG files/); + } catch (error) { + expect(error.message).to.match(/No SVG files/); } }); it('should show message when folder does not consists any svg files', async () => { - replaceConsoleError(); try { await runProgram(['--folder', path.resolve(__dirname, 'testFolderWithNoSvg'), '--quiet']) - } catch {} finally { - restoreConsoleError(); - expect(output).to.match(/No SVG files have been found/); + } catch (error) { + expect(error.message).to.match(/No SVG files have been found/); } }); diff --git a/test/config/_index.js b/test/config/_index.js index 060d93286..ea982883e 100644 --- a/test/config/_index.js +++ b/test/config/_index.js @@ -243,6 +243,17 @@ describe('config', function() { expect(error.message).to.match(/Invalid config file/); } }); + it('handles config errors properly', async () => { + try { + await loadConfig( + null, + path.join(process.cwd(), './test/config/fixtures/invalid/config'), + ); + expect.fail('Config is loaded successfully'); + } catch (error) { + expect(error.message).to.match(/plugins is not defined/); + } + }); }); }); diff --git a/test/config/fixtures/invalid/svgo.config.js b/test/config/fixtures/invalid/svgo.config.js new file mode 100644 index 000000000..84ff60097 --- /dev/null +++ b/test/config/fixtures/invalid/svgo.config.js @@ -0,0 +1 @@ +module.exports = { plugins }; From 7ad35ac4ef4c42efb9d1d115a1feeaa2a4aa1bcf Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 19 Feb 2021 02:43:51 +0300 Subject: [PATCH 2/5] getStats -> isFile --- lib/svgo-node.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/svgo-node.js b/lib/svgo-node.js index f374711aa..661dffadc 100644 --- a/lib/svgo-node.js +++ b/lib/svgo-node.js @@ -23,11 +23,12 @@ const importConfig = async configFile => { } }; -const getStats = async (file) => { +const isFile = async (file) => { try { - return await fs.promises.stat(file); + const stats = await fs.promises.stat(file); + return stats.isFile(); } catch { - return null; + return false; } } @@ -42,8 +43,7 @@ const loadConfig = async (configFile, cwd = process.cwd()) => { let dir = cwd; while (true) { const file = path.join(dir, "svgo.config.js"); - const stats = await getStats(file); - if (stats && stats.isFile()) { + if (await isFile(file)) { return await importConfig(file); } const parent = path.dirname(dir); From 10f58a82eb57b4d6bd556eed278afafafba75ff4 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 19 Feb 2021 12:00:32 +0300 Subject: [PATCH 3/5] Check module before requiring --- lib/svgo-node.js | 18 ++++++++---------- test/config/_index.js | 10 ++++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/svgo-node.js b/lib/svgo-node.js index 661dffadc..676b61eb2 100644 --- a/lib/svgo-node.js +++ b/lib/svgo-node.js @@ -10,17 +10,15 @@ const { const importConfig = async configFile => { try { - const config = require(configFile); - if (config == null || typeof config !== 'object' || Array.isArray(config)) { - throw Error(`Invalid config file "${configFile}"`); - } - return config; - } catch (error) { - if (error.code === 'MODULE_NOT_FOUND') { - return null; - } - throw error; + await fs.promises.access(configFile); + } catch { + return null; + } + const config = require(configFile); + if (config == null || typeof config !== 'object' || Array.isArray(config)) { + throw Error(`Invalid config file "${configFile}"`); } + return config; }; const isFile = async (file) => { diff --git a/test/config/_index.js b/test/config/_index.js index ea982883e..37dc291c2 100644 --- a/test/config/_index.js +++ b/test/config/_index.js @@ -254,6 +254,16 @@ describe('config', function() { expect(error.message).to.match(/plugins is not defined/); } }); + it('handles MODULE_NOT_FOUND properly', async () => { + try { + await loadConfig( + path.join(process.cwd(), './test/config/fixtures/module-not-found.js'), + ); + expect.fail('Config is loaded successfully'); + } catch (error) { + expect(error.message).to.match(/Cannot find module 'unknown-module'/); + } + }); }); }); From 30052ce48c98f83c84bf406611897a05b4daa77e Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 19 Feb 2021 12:02:18 +0300 Subject: [PATCH 4/5] Commit fixture --- input.svg | 3 + output.svg | 1 + svgo2.config.js | 144 +++++++++++++++++++++++ test/config/fixtures/module-not-found.js | 2 + 4 files changed, 150 insertions(+) create mode 100644 input.svg create mode 100644 output.svg create mode 100644 svgo2.config.js create mode 100644 test/config/fixtures/module-not-found.js diff --git a/input.svg b/input.svg new file mode 100644 index 000000000..f38da92b4 --- /dev/null +++ b/input.svg @@ -0,0 +1,3 @@ + + test + diff --git a/output.svg b/output.svg new file mode 100644 index 000000000..5edef2c40 --- /dev/null +++ b/output.svg @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/svgo2.config.js b/svgo2.config.js new file mode 100644 index 000000000..6084999b7 --- /dev/null +++ b/svgo2.config.js @@ -0,0 +1,144 @@ +'use strict' + +const { extendDefaultPlugins } = require('svgo') +console.log('fuck'); +throw Error('fuck'); + +module.exports = { + multipass: true, + js2svg: { + pretty: true, + indent: 2 + }, + plugins: [ + /* + { + name: 'cleanupAttrs' + }, + { + name: 'cleanupEnableBackground' + }, + { + name: 'cleanupIDs' + }, + { + name: 'cleanupListOfValues' + }, + { + name: 'cleanupNumericValues' + }, + { + name: 'collapseGroups' + }, + { + name: 'convertColors' + }, + { + name: 'convertPathData', + params: { + noSpaceAfterFlags: false + } + }, + { + name: 'convertShapeToPath' + }, + { + name: 'convertStyleToAttrs' + }, + { + name: 'convertTransform' + }, + { + name: 'inlineStyles' + }, + { + name: 'mergePaths', + params: { + noSpaceAfterFlags: false + } + }, + { + name: 'minifyStyles' + }, + { + name: 'moveElemsAttrsToGroup' + }, + { + name: 'moveGroupAttrsToElems' + }, + { + name: 'removeAttrs', + params: { + attrs: [ + 'data-name', + 'fill', + 'clip-rule' + ] + } + }, + { + name: 'removeComments' + }, + { + name: 'removeDesc' + }, + { + name: 'removeDoctype' + }, + { + name: 'removeEditorsNSData' + }, + { + name: 'removeEmptyAttrs' + }, + { + name: 'removeEmptyContainers' + }, + { + name: 'removeEmptyText' + }, + { + name: 'removeHiddenElems' + }, + { + name: 'removeMetadata' + }, + { + name: 'removeNonInheritableGroupAttrs' + }, + { + name: 'removeTitle' + }, + { + name: 'removeUnknownsAndDefaults', + params: { + keepRoleAttr: true + } + }, + { + name: 'removeUnusedNS' + }, + { + name: 'removeUselessDefs' + }, + { + name: 'removeUselessStrokeAndFill' + }, + */ + { + name: 'removeViewBox', + }, + /* + { + name: 'removeXMLNS', + active: false + }, + { + name: 'removeXMLProcInst' + }, + { + name: 'sortAttrs' + } + */ + ] +} diff --git a/test/config/fixtures/module-not-found.js b/test/config/fixtures/module-not-found.js new file mode 100644 index 000000000..32a1fb962 --- /dev/null +++ b/test/config/fixtures/module-not-found.js @@ -0,0 +1,2 @@ +require('unknown-module'); +module.exports = {} From ff447e8d2c9a5c49eff702a3db690fd6b42c3dcd Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 19 Feb 2021 12:05:36 +0300 Subject: [PATCH 5/5] Remove files --- input.svg | 3 - output.svg | 1 - svgo2.config.js | 144 ------------------------------------------------ 3 files changed, 148 deletions(-) delete mode 100644 input.svg delete mode 100644 output.svg delete mode 100644 svgo2.config.js diff --git a/input.svg b/input.svg deleted file mode 100644 index f38da92b4..000000000 --- a/input.svg +++ /dev/null @@ -1,3 +0,0 @@ - - test - diff --git a/output.svg b/output.svg deleted file mode 100644 index 5edef2c40..000000000 --- a/output.svg +++ /dev/null @@ -1 +0,0 @@ -test \ No newline at end of file diff --git a/svgo2.config.js b/svgo2.config.js deleted file mode 100644 index 6084999b7..000000000 --- a/svgo2.config.js +++ /dev/null @@ -1,144 +0,0 @@ -'use strict' - -const { extendDefaultPlugins } = require('svgo') -console.log('fuck'); -throw Error('fuck'); - -module.exports = { - multipass: true, - js2svg: { - pretty: true, - indent: 2 - }, - plugins: [ - /* - { - name: 'cleanupAttrs' - }, - { - name: 'cleanupEnableBackground' - }, - { - name: 'cleanupIDs' - }, - { - name: 'cleanupListOfValues' - }, - { - name: 'cleanupNumericValues' - }, - { - name: 'collapseGroups' - }, - { - name: 'convertColors' - }, - { - name: 'convertPathData', - params: { - noSpaceAfterFlags: false - } - }, - { - name: 'convertShapeToPath' - }, - { - name: 'convertStyleToAttrs' - }, - { - name: 'convertTransform' - }, - { - name: 'inlineStyles' - }, - { - name: 'mergePaths', - params: { - noSpaceAfterFlags: false - } - }, - { - name: 'minifyStyles' - }, - { - name: 'moveElemsAttrsToGroup' - }, - { - name: 'moveGroupAttrsToElems' - }, - { - name: 'removeAttrs', - params: { - attrs: [ - 'data-name', - 'fill', - 'clip-rule' - ] - } - }, - { - name: 'removeComments' - }, - { - name: 'removeDesc' - }, - { - name: 'removeDoctype' - }, - { - name: 'removeEditorsNSData' - }, - { - name: 'removeEmptyAttrs' - }, - { - name: 'removeEmptyContainers' - }, - { - name: 'removeEmptyText' - }, - { - name: 'removeHiddenElems' - }, - { - name: 'removeMetadata' - }, - { - name: 'removeNonInheritableGroupAttrs' - }, - { - name: 'removeTitle' - }, - { - name: 'removeUnknownsAndDefaults', - params: { - keepRoleAttr: true - } - }, - { - name: 'removeUnusedNS' - }, - { - name: 'removeUselessDefs' - }, - { - name: 'removeUselessStrokeAndFill' - }, - */ - { - name: 'removeViewBox', - }, - /* - { - name: 'removeXMLNS', - active: false - }, - { - name: 'removeXMLProcInst' - }, - { - name: 'sortAttrs' - } - */ - ] -}