From 3d8231722190b45bb5641c639a4607d620a9c97d Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 29 Sep 2024 22:56:07 -0700 Subject: [PATCH] Load svgo version from project --- packages/core/integration-tests/test/html.js | 72 +++++++ .../integration/svgo-config/svgo.config.js | 14 +- packages/core/integration-tests/test/svg.js | 79 +++++++- packages/core/utils/src/index.js | 1 + packages/core/utils/src/svgo.js | 50 +++++ packages/optimizers/htmlnano/package.json | 8 +- .../htmlnano/src/HTMLNanoOptimizer.js | 191 +++++++++++++++--- packages/optimizers/svgo/package.json | 6 +- packages/optimizers/svgo/src/SVGOOptimizer.js | 115 +++++++++-- yarn.lock | 2 +- 10 files changed, 470 insertions(+), 68 deletions(-) create mode 100644 packages/core/utils/src/svgo.js diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js index bcd5ecf8295..b862b2f98a2 100644 --- a/packages/core/integration-tests/test/html.js +++ b/packages/core/integration-tests/test/html.js @@ -14,6 +14,7 @@ import { fsFixture, } from '@parcel/test-utils'; import path from 'path'; +import Logger from '@parcel/logger'; describe('html', function () { beforeEach(async () => { @@ -679,6 +680,77 @@ describe('html', function () { ); }); + it('should detect the version of SVGO to use', async function () { + // Test is outside parcel so that svgo is not already installed. + await fsFixture(overlayFS, '/')` + htmlnano-svgo-version + index.html: + + + + + + + + .htmlnanorc: + { + "minifySvg": { + "full": true + } + } + + yarn.lock: + `; + + let messages = []; + let loggerDisposable = Logger.onLog(message => { + if (message.level !== 'verbose') { + messages.push(message); + } + }); + + try { + await bundle(path.join('/htmlnano-svgo-version/index.html'), { + inputFS: overlayFS, + defaultTargetOptions: { + shouldOptimize: true, + }, + shouldAutoinstall: false, + }); + } catch (err) { + // autoinstall is disabled + assert.equal( + err.diagnostics[0].message, + 'Could not resolve module "svgo" from "/htmlnano-svgo-version/index"', + ); + } + + loggerDisposable.dispose(); + assert( + messages[0].diagnostics[0].message.startsWith( + 'Detected deprecated SVGO v2 options in', + ), + ); + assert.deepEqual(messages[0].diagnostics[0].codeFrames, [ + { + filePath: path.normalize('/htmlnano-svgo-version/.htmlnanorc'), + codeHighlights: [ + { + message: undefined, + start: { + line: 3, + column: 5, + }, + end: { + line: 3, + column: 16, + }, + }, + ], + }, + ]); + }); + it('should not minify default values inside HTML in production mode', async function () { let inputFile = path.join( __dirname, diff --git a/packages/core/integration-tests/test/integration/svgo-config/svgo.config.js b/packages/core/integration-tests/test/integration/svgo-config/svgo.config.js index d784474232d..89d47b4aced 100644 --- a/packages/core/integration-tests/test/integration/svgo-config/svgo.config.js +++ b/packages/core/integration-tests/test/integration/svgo-config/svgo.config.js @@ -1,10 +1,12 @@ -const { extendDefaultPlugins } = require('svgo'); - module.exports = { - plugins: extendDefaultPlugins([ + plugins: [ { - name: 'removeComments', - active: false + name: 'preset-default', + params: { + overrides: { + removeComments: false + } + } } - ]) + ] } diff --git a/packages/core/integration-tests/test/svg.js b/packages/core/integration-tests/test/svg.js index 6eebd2c937f..69b484c75ac 100644 --- a/packages/core/integration-tests/test/svg.js +++ b/packages/core/integration-tests/test/svg.js @@ -1,6 +1,14 @@ import assert from 'assert'; -import {assertBundles, bundle, distDir, outputFS} from '@parcel/test-utils'; +import { + assertBundles, + bundle, + distDir, + outputFS, + overlayFS, + fsFixture, +} from '@parcel/test-utils'; import path from 'path'; +import Logger from '@parcel/logger'; describe('svg', function () { it('should support bundling SVG', async () => { @@ -126,6 +134,73 @@ describe('svg', function () { assert(file.includes('comment')); }); + it('should detect the version of SVGO to use', async function () { + // Test is outside parcel so that svgo is not already installed. + await fsFixture(overlayFS, '/')` + svgo-version + icon.svg: + + + index.html: + + + svgo.config.json: + { + "full": true + } + + yarn.lock: + `; + + let messages = []; + let loggerDisposable = Logger.onLog(message => { + if (message.level !== 'verbose') { + messages.push(message); + } + }); + + try { + await bundle(path.join('/svgo-version/index.html'), { + inputFS: overlayFS, + defaultTargetOptions: { + shouldOptimize: true, + }, + shouldAutoinstall: false, + }); + } catch (err) { + // autoinstall is disabled + assert.equal( + err.diagnostics[0].message, + 'Could not resolve module "svgo" from "/svgo-version/index"', + ); + } + + loggerDisposable.dispose(); + assert( + messages[0].diagnostics[0].message.startsWith( + 'Detected deprecated SVGO v2 options in', + ), + ); + assert.deepEqual(messages[0].diagnostics[0].codeFrames, [ + { + filePath: path.normalize('/svgo-version/svgo.config.json'), + codeHighlights: [ + { + message: undefined, + start: { + line: 2, + column: 3, + }, + end: { + line: 2, + column: 14, + }, + }, + ], + }, + ]); + }); + it('should detect xml-stylesheet processing instructions', async function () { let b = await bundle( path.join(__dirname, '/integration/svg-xml-stylesheet/img.svg'), @@ -222,7 +297,7 @@ describe('svg', function () { const svg = await outputFS.readFile(path.join(distDir, 'img.svg'), 'utf8'); - assert(svg.includes('')); + assert(svg.includes('style="fill:red"')); }); it('should be in separate bundles', async function () { diff --git a/packages/core/utils/src/index.js b/packages/core/utils/src/index.js index ee499b0a3e6..57bd8598d10 100644 --- a/packages/core/utils/src/index.js +++ b/packages/core/utils/src/index.js @@ -86,3 +86,4 @@ export { remapSourceLocation, } from './sourcemap'; export {default as stripAnsi} from 'strip-ansi'; +export {detectSVGOVersion} from './svgo'; diff --git a/packages/core/utils/src/svgo.js b/packages/core/utils/src/svgo.js new file mode 100644 index 00000000000..2f20b2e10a5 --- /dev/null +++ b/packages/core/utils/src/svgo.js @@ -0,0 +1,50 @@ +// @flow +export function detectSVGOVersion( + config: any, +): {|version: 3|} | {|version: 2, path: string|} { + if (!config) { + return {version: 3}; + } + + // These options were removed in v2. + if (config.full != null || config.svg2js != null) { + return {version: 2, path: config.full != null ? '/full' : '/svg2js'}; + } + + if (Array.isArray(config.plugins)) { + // Custom plugins in v2 had additional (required) fields that don't exist anymore. + let v2Plugin = config.plugins.findIndex( + p => p?.type != null || (p?.fn && p?.params != null), + ); + if (v2Plugin !== -1) { + let field = config.plugins[v2Plugin].type != null ? 'type' : 'params'; + return {version: 2, path: `/plugins/${v2Plugin}/${field}`}; + } + + // the cleanupIDs plugin lost the prefix option in v3. + let cleanupIdsIndex = config.plugins.findIndex( + p => p?.name === 'cleanupIDs', + ); + let cleanupIDs = + cleanupIdsIndex !== -1 ? config.plugins[cleanupIdsIndex] : null; + if (cleanupIDs?.params?.prefix != null) { + return {version: 2, path: `/plugins/${cleanupIdsIndex}/params/prefix`}; + } + + // Automatically migrate some options from SVGO 2 config files. + config.plugins = config.plugins.filter(p => p?.active !== false); + + for (let i = 0; i < config.plugins.length; i++) { + let p = config.plugins[i]; + if (p === 'cleanupIDs') { + config.plugins[i] = 'cleanupIds'; + } + + if (p?.name === 'cleanupIDs') { + config.plugins[i].name = 'cleanupIds'; + } + } + } + + return {version: 3}; +} diff --git a/packages/optimizers/htmlnano/package.json b/packages/optimizers/htmlnano/package.json index 505a182bcd8..56f35527532 100644 --- a/packages/optimizers/htmlnano/package.json +++ b/packages/optimizers/htmlnano/package.json @@ -20,10 +20,14 @@ "parcel": "^2.12.0" }, "dependencies": { + "@parcel/diagnostic": "2.12.0", "@parcel/plugin": "2.12.0", + "@parcel/utils": "2.12.0", "htmlnano": "^2.0.0", "nullthrows": "^1.1.1", - "posthtml": "^0.16.5", - "svgo": "^2.4.0" + "posthtml": "^0.16.5" + }, + "devDependencies": { + "svgo": "^3.3.2" } } diff --git a/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js b/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js index 32c2713f071..862c157f4f4 100644 --- a/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js +++ b/packages/optimizers/htmlnano/src/HTMLNanoOptimizer.js @@ -2,13 +2,19 @@ import type {PostHTMLNode} from 'posthtml'; import htmlnano from 'htmlnano'; +import { + md, + generateJSONCodeHighlights, + errorToDiagnostic, +} from '@parcel/diagnostic'; import {Optimizer} from '@parcel/plugin'; +import {detectSVGOVersion} from '@parcel/utils'; import posthtml from 'posthtml'; import path from 'path'; import {SVG_ATTRS, SVG_TAG_NAMES} from './svgMappings'; export default (new Optimizer({ - async loadConfig({config, options}) { + async loadConfig({config, options, logger}) { let userConfig = await config.getConfigFrom( path.join(options.projectRoot, 'index.html'), [ @@ -26,9 +32,72 @@ export default (new Optimizer({ }, ); - return userConfig?.contents; + let contents = userConfig?.contents; + + // See if svgo is already installed. + let resolved; + try { + resolved = await options.packageManager.resolve( + 'svgo', + path.join(options.projectRoot, 'index'), + {shouldAutoInstall: false}, + ); + } catch (err) { + // ignore. + } + + // If so, use the existing installed version. + let svgoVersion = 3; + if (resolved) { + if (resolved.pkg?.version) { + svgoVersion = parseInt(resolved.pkg.version); + } + } else if (contents?.minifySvg) { + // Otherwise try to detect the version based on the config file. + let v = detectSVGOVersion(contents.minifySvg); + if (userConfig != null && v.version === 2) { + logger.warn({ + message: md`Detected deprecated SVGO v2 options in ${path.relative( + process.cwd(), + userConfig.filePath, + )}`, + codeFrames: [ + { + filePath: userConfig.filePath, + codeHighlights: + path.basename(userConfig.filePath) === '.htmlnanorc' || + path.extname(userConfig.filePath) === '.json' + ? generateJSONCodeHighlights( + await options.inputFS.readFile( + userConfig.filePath, + 'utf8', + ), + [ + { + key: `${ + path.basename(userConfig.filePath) === + 'package.json' + ? '/htmlnano' + : '' + }/minifySvg${v.path}`, + }, + ], + ) + : [], + }, + ], + }); + } + + svgoVersion = v.version; + } + + return { + contents, + svgoVersion, + }; }, - async optimize({bundle, contents, map, config}) { + async optimize({bundle, contents, map, config, options, logger}) { if (!bundle.env.shouldOptimize) { return {contents, map}; } @@ -39,7 +108,7 @@ export default (new Optimizer({ ); } - const clonedConfig = config || {}; + const clonedConfig = config.contents || {}; // $FlowFixMe const presets = htmlnano.presets; @@ -54,35 +123,11 @@ export default (new Optimizer({ // minified before they are re-inserted by the packager. minifyJs: false, minifyCss: false, - minifySvg: { - plugins: [ - { - name: 'preset-default', - params: { - overrides: { - // Copied from htmlnano defaults. - collapseGroups: false, - convertShapeToPath: false, - // Additional defaults to preserve accessibility information. - removeTitle: false, - removeDesc: false, - removeUnknownsAndDefaults: { - keepAriaAttrs: true, - keepRoleAttr: true, - }, - // Do not minify ids or remove unreferenced elements in - // inline SVGs because they could actually be referenced - // by a separate inline SVG. - cleanupIDs: false, - }, - }, - }, - // XML namespaces are not required in HTML. - 'removeXMLNS', - ], - }, ...(preset || {}), ...clonedConfig, + // Never use htmlnano's builtin svgo transform. + // We need to control the version of svgo that is used. + minifySvg: false, // TODO: Uncomment this line once we update htmlnano, new version isn't out yet // skipConfigLoading: true, }; @@ -90,8 +135,17 @@ export default (new Optimizer({ let plugins = [htmlnano(htmlNanoConfig)]; // $FlowFixMe - if (htmlNanoConfig.minifySvg !== false) { - plugins.unshift(mapSVG); + if (clonedConfig.minifySvg !== false) { + plugins.push(mapSVG); + plugins.push(tree => + minifySvg( + tree, + options, + config.svgoVersion, + clonedConfig.minifySvg, + logger, + ), + ); } return { @@ -158,3 +212,74 @@ function mapSVG( return node; } + +async function minifySvg(tree, options, svgoVersion, svgoOptions, logger) { + let svgNodes = []; + tree.match({tag: 'svg'}, node => { + svgNodes.push(node); + return node; + }); + + if (!svgNodes.length) { + return tree; + } + + const svgo = await options.packageManager.require( + 'svgo', + path.join(options.projectRoot, 'index'), + { + range: `^${svgoVersion}`, + saveDev: true, + shouldAutoInstall: options.shouldAutoInstall, + }, + ); + + let opts = svgoOptions; + if (!svgoOptions) { + let cleanupIds: string = svgoVersion === 2 ? 'cleanupIDs' : 'cleanupIds'; + opts = { + plugins: [ + { + name: 'preset-default', + params: { + overrides: { + // Copied from htmlnano defaults. + collapseGroups: false, + convertShapeToPath: false, + // Additional defaults to preserve accessibility information. + removeTitle: false, + removeDesc: false, + removeUnknownsAndDefaults: { + keepAriaAttrs: true, + keepRoleAttr: true, + }, + // Do not minify ids or remove unreferenced elements in + // inline SVGs because they could actually be referenced + // by a separate inline SVG. + [cleanupIds]: false, + }, + }, + }, + // XML namespaces are not required in HTML. + 'removeXMLNS', + ], + }; + } + + for (let node of svgNodes) { + let svgStr = tree.render(node, { + closingSingleTag: 'slash', + quoteAllAttributes: true, + }); + try { + let result = svgo.optimize(svgStr, opts); + node.tag = false; + node.attrs = {}; + node.content = [result.data]; + } catch (error) { + logger.warn(errorToDiagnostic(error)); + } + } + + return tree; +} diff --git a/packages/optimizers/svgo/package.json b/packages/optimizers/svgo/package.json index a068cfe3d3b..da54141c7bf 100644 --- a/packages/optimizers/svgo/package.json +++ b/packages/optimizers/svgo/package.json @@ -22,7 +22,9 @@ "dependencies": { "@parcel/diagnostic": "2.12.0", "@parcel/plugin": "2.12.0", - "@parcel/utils": "2.12.0", - "svgo": "^2.4.0" + "@parcel/utils": "2.12.0" + }, + "devDependencies": { + "svgo": "^3.3.2" } } diff --git a/packages/optimizers/svgo/src/SVGOOptimizer.js b/packages/optimizers/svgo/src/SVGOOptimizer.js index 22136c43bf7..d7ccb1855c5 100644 --- a/packages/optimizers/svgo/src/SVGOOptimizer.js +++ b/packages/optimizers/svgo/src/SVGOOptimizer.js @@ -1,13 +1,16 @@ // @flow import {Optimizer} from '@parcel/plugin'; -import ThrowableDiagnostic from '@parcel/diagnostic'; -import {blobToString} from '@parcel/utils'; - -import * as svgo from 'svgo'; +import ThrowableDiagnostic, { + errorToDiagnostic, + md, + generateJSONCodeHighlights, +} from '@parcel/diagnostic'; +import {blobToString, detectSVGOVersion} from '@parcel/utils'; +import path from 'path'; export default (new Optimizer({ - async loadConfig({config}) { + async loadConfig({config, logger, options}) { let configFile = await config.getConfig([ 'svgo.config.js', 'svgo.config.cjs', @@ -15,33 +18,101 @@ export default (new Optimizer({ 'svgo.config.json', ]); - return configFile?.contents; + // See if svgo is already installed. + let resolved; + try { + resolved = await options.packageManager.resolve( + 'svgo', + path.join(options.projectRoot, 'index'), + {shouldAutoInstall: false}, + ); + } catch (err) { + // ignore. + } + + // If so, use the existing installed version. + let version = 3; + if (resolved) { + if (resolved.pkg?.version) { + version = parseInt(resolved.pkg.version); + } + } else { + // Otherwise try to detect the version based on the config file. + let v = detectSVGOVersion(configFile?.contents); + if (configFile != null && v.version === 2) { + logger.warn({ + message: md`Detected deprecated SVGO v2 options in ${path.relative( + process.cwd(), + configFile.filePath, + )}`, + codeFrames: [ + { + filePath: configFile.filePath, + codeHighlights: + path.extname(configFile.filePath) === '.json' + ? generateJSONCodeHighlights( + await options.inputFS.readFile( + configFile.filePath, + 'utf8', + ), + [{key: v.path}], + ) + : [], + }, + ], + }); + } + + version = v.version; + } + + return { + contents: configFile?.contents, + version, + }; }, - async optimize({bundle, contents, config}) { + async optimize({bundle, contents, config, options}) { if (!bundle.env.shouldOptimize) { return {contents}; } + const svgo = await options.packageManager.require( + 'svgo', + path.join(options.projectRoot, 'index'), + { + range: `^${config.version}`, + saveDev: true, + shouldAutoInstall: options.shouldAutoInstall, + }, + ); + let code = await blobToString(contents); - let result = svgo.optimize(code, { - plugins: [ - { - name: 'preset-default', - params: { - overrides: { - // Removing ids could break SVG sprites. - cleanupIDs: false, - //