Multipass, addClassesToSVGElement duplicating classes

[Matt-Butler]

const svgoConfig = (isClass1, isClass2, isClass3) => {
  const classes = classNames({ 'class1': isClass1 }, { 'class2': isClass2 }, { 'class3': isClass3 });

  return {
    multipass: true,
    floatPrecision: 2,
    plugins: [
      {
        mergePaths: false,
      },
      {
        removeTitle: true,
      },
      {
        addAttributesToSVGElement: {
          attribute: 'aria-hidden="true"',
        },
      },
      {
        addClassesToSVGElement: {
          className: classes,
        },
      },
    ],
  };
};

The class names are duplicated on the SVG.

[GreLI]

Yep, multipass literally runs SVGO several times with all its plugins. Probably, it worth consider for class duplication checking before applying. /cc @AprilArcus.

[AprilArcus]

Thanks, I'll take this issue.

[Matt-Butler]

Thanks @AprilArcus @GreLI

[mdmoreau]

@AprilArcus @GreLI Has there been any progress on this issue? I'm running into the same sort of problem when using multipass with prefixIds or when using prefixIds on a file that has already had classes prefixed.

[strarsis]

@mdmoreau: I am just writing some tests in svgo for multipass and I noticed that addClassesToSVGElement with its current code correctly adds classes only once.
addAttributesToSVGElement isn't idempotent though as I noticed during testing.

[strarsis]

@Matt-Butler: So I wrote some tests for addAttributesToSVGElement and addClassesToSVGElement plugin and they both work with multipass...

[mdmoreau]

@strarsis I'm still seeing the same behavior on 1.3.2 unfortunately. Setup a demo at https://github.com/mdmoreau/svgo-demo where an additional class prefix gets added each time SVGO is run.

[strarsis]

@mdmoreau: Yes, there is a bug that was uncovered with multipass tests and there is a yet unmerged PR to fix it, see #1177.

[mdmoreau]

@strarsis Tried that on my demo, and noticed a couple of things:

1. IDs are no longer prefixed (worked correctly before)
2. The class prefixes don't get duplicated by multipass, but they do get duplicated when re-running the same file through SVGO again

[strarsis]

@mdmoreau: That they are still prefixed multiple times when they are passed multiple times to svgo is an intended behavior.

[mdmoreau]

@strarsis Oh okay - wasn't aware of that. Was hoping that there was some sort of check to see if the prefix existed before re-adding it. Is there a reason something like that couldn't be added?

[strarsis]

@mdmoreau: The reason for all the issues is that the gulp plugin for svgo enables the multipass feature of svgo by default. The SVG file passed to that plugin, only once, is passed multiple times through all the enabled plugins, including the prefixIds plugin. So when you are using the gulp plugin for svgo, the SVG files you pass to it are passed multiple times through all the plugins, including prefixIds.
A fix has been added to svgo some days ago that sadly required another fix right afterwards because of the missing tests in svgo for multipass, it caused issues when no info object is passed to svgo. See this PR: #1177

[TrySound]

addClassesToSVGElement does not add duplicated classes for some time. addAttributesToSVGElement produces duplicated attribute because the whole aria-hidden="true" string is treated as key.

{
    'aria-hidden': 'true',
    'aria-hidden="true"': undefined
}

Better use object { 'aria-hidden': 'true' '}