Skip to content

Commit

Permalink
refactor: create hasScripts util (#1809)
Browse files Browse the repository at this point in the history
Introduces a hasScript utility which can be reused to consistently check for scripts, whereas before different plugins performed this check differently.
  • Loading branch information
SethFalco committed Oct 22, 2023
1 parent e529c66 commit 3966c10
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 10 deletions.
38 changes: 38 additions & 0 deletions lib/svgo/tools.js
@@ -1,10 +1,13 @@
'use strict';

/**
* @typedef {import('../../lib/types').XastElement} XastElement
* @typedef {import('../types').PathDataCommand} PathDataCommand
* @typedef {import('../types').DataUri} DataUri
*/

const { attrsGroups } = require('../../plugins/_collections');

/**
* Encode plain SVG data string into Data URI string.
*
Expand Down Expand Up @@ -136,3 +139,38 @@ const removeLeadingZero = (num) => {
return strNum;
};
exports.removeLeadingZero = removeLeadingZero;

/**
* If the current node contains any scripts. This does not check parents or
* children of the node, only the properties and attributes of the node itself.
*
* @param {XastElement} node Current node to check against.
* @returns {boolean} If the current node contains scripts.
*/
const hasScripts = (node) => {
if (node.name === 'script' && node.children.length !== 0) {
return true;
}

if (node.name === 'a') {
const hasJsLinks = Object.entries(node.attributes).some(
([attrKey, attrValue]) =>
(attrKey === 'href' || attrKey.endsWith(':href')) &&
attrValue != null &&
attrValue.trimStart().startsWith('javascript:')
);

if (hasJsLinks) {
return true;
}
}

const eventAttrs = [
...attrsGroups.animationEvent,
...attrsGroups.graphicalEvent,
...attrsGroups.documentEvent,
];

return eventAttrs.some((attr) => node.attributes[attr] != null);
};
exports.hasScripts = hasScripts;
9 changes: 5 additions & 4 deletions plugins/cleanupIds.js
Expand Up @@ -5,6 +5,7 @@
*/

const { visitSkip } = require('../lib/xast.js');
const { hasScripts } = require('../lib/svgo/tools');
const { referencesProps } = require('./_collections.js');

exports.name = 'cleanupIds';
Expand Down Expand Up @@ -154,11 +155,11 @@ exports.fn = (_root, params) => {
return {
element: {
enter: (node) => {
if (force == false) {
// deoptimize if style or script elements are present
if (!force) {
// deoptimize if style or scripts are present
if (
(node.name === 'style' || node.name === 'script') &&
node.children.length !== 0
(node.name === 'style' && node.children.length !== 0) ||
hasScripts(node)
) {
deoptimized = true;
return;
Expand Down
8 changes: 3 additions & 5 deletions plugins/minifyStyles.js
Expand Up @@ -7,6 +7,7 @@

const csso = require('csso');
const { detachNodeFromParent } = require('../lib/xast');
const { hasScripts } = require('../lib/svgo/tools');

exports.name = 'minifyStyles';
exports.description = 'minifies styles and removes unused styles';
Expand Down Expand Up @@ -39,7 +40,7 @@ exports.fn = (_root, { usage, ...params }) => {

/**
* Force to use usage data even if it unsafe. For example, the document
* contains <script> or in attributes..
* contains scripts or in attributes..
*/
let forceUsageDeoptimized = false;

Expand All @@ -60,10 +61,7 @@ exports.fn = (_root, { usage, ...params }) => {
element: {
enter: (node, parentNode) => {
// detect deoptimisations
if (
node.name === 'script' ||
Object.keys(node.attributes).some((name) => name.startsWith('on'))
) {
if (hasScripts(node)) {
deoptimized = true;
}

Expand Down
3 changes: 2 additions & 1 deletion plugins/removeUselessStrokeAndFill.js
Expand Up @@ -2,6 +2,7 @@

const { visit, visitSkip, detachNodeFromParent } = require('../lib/xast.js');
const { collectStylesheet, computeStyle } = require('../lib/style.js');
const { hasScripts } = require('../lib/svgo/tools.js');
const { elemsGroups } = require('./_collections.js');

exports.name = 'removeUselessStrokeAndFill';
Expand All @@ -26,7 +27,7 @@ exports.fn = (root, params) => {
visit(root, {
element: {
enter: (node) => {
if (node.name === 'style' || node.name === 'script') {
if (node.name === 'style' || hasScripts(node)) {
hasStyleOrScript = true;
}
},
Expand Down

0 comments on commit 3966c10

Please sign in to comment.