patrickxchong / eleventy-plugin-svg-sprite

A high performance Eleventy universal plugin that compiles a directory of SVG files into a single SVG Sprite and adds shortcodes to embed SVG Sprite and SVG content in Eleventy templates.
MIT License
39 stars 6 forks source link

`spriteConfig` options must include all defaults #14

Closed tedw closed 1 year ago

tedw commented 1 year ago

It seems changing any defaults in spriteConfig causes all of the options to be overwritten. I was just trying to make a change to SVGO’s settings but had to redefine everything listed in options.js. Below is an explanation of my original issue and how I went about fixing it.


I noticed any <g> tags in my SVGs were removed in the sprite, with their attributes applied to all children.

Here’s an example:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" preserveAspectRatio="xMidYMid meet">
  <g fill="currentColor" class="foo bar">
    <path fill="var(--icon-fill)" d="M19.5 8.66v2.64h-7.97v8.2h-3.1v-8.2H.5V8.66h7.93V.5h3.1v8.16z"/>
  </g>
</svg>

Here’s the inline sprite output:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <symbol viewBox="0 0 20 20" id="svg-plus-outline" xmlns="http://www.w3.org/2000/svg">
    <path fill="var(--icon-fill)" d="M19.5 8.66v2.64h-7.97v8.2h-3.1v-8.2H.5V8.66h7.93V.5h3.1v8.16z" class="dfoo dbar" />
  </symbol>
</svg>

Turns out this was because svg-sprite uses the default SVGO settings, which set collapseGroups to true.

I first tried adding this config info but got an error when booting Eleventy:

config.addPlugin(SVGSpritePlugin, {
  path: './src/assets/icons/sprite',
  spriteConfig: {
    shape: {
      transform: [{
        svgo: {
          plugins: [{
            name: 'preset-default',
            params: {
              overrides: {
                collapseGroups: false
              }
            }
          }]
        }
      }]
    }
  }
});

Error message:

[start:eleventy] [11ty] Problem writing Eleventy templates: (more in DEBUG output)
[start:eleventy] [11ty] Cannot read properties of undefined (reading 'sprite') (via TypeError)
[start:eleventy] [11ty]
[start:eleventy] [11ty] Original error stack trace: TypeError: Cannot read properties of undefined (reading 'sprite')
[start:eleventy] [11ty]     at /Users/twhitehead/Sites/malala-report-cards/node_modules/eleventy-plugin-svg-sprite/src/SVGSprite.js:65:33
[start:eleventy] [11ty]     at SVGSpriter._compile (/Users/[redacted]/node_modules/svg-sprite/lib/svg-sprite.js:260:13)
[start:eleventy] [11ty]     at SVGSpriter.compile (/Users/[redacted]/node_modules/svg-sprite/lib/svg-sprite.js:221:10)
[start:eleventy] [11ty]     at /Users/[redacted]/node_modules/eleventy-plugin-svg-sprite/src/SVGSprite.js:61:17
[start:eleventy] [11ty]     at new Promise (<anonymous>)
[start:eleventy] [11ty]     at compileSprite (/Users/[redacted]/node_modules/eleventy-plugin-svg-sprite/src/SVGSprite.js:60:14)
[start:eleventy] [11ty]     at SVGSprite.compile (/Users/[redacted]/node_modules/eleventy-plugin-svg-sprite/src/SVGSprite.js:71:26)
[start:eleventy] [11ty]     at async AsyncEventEmitter.<anonymous> (/Users/[redacted]/node_modules/eleventy-plugin-svg-sprite/.eleventy.js:26:52)
[start:eleventy] [11ty]     at async Promise.all (index 0)
[start:eleventy] [11ty]     at async Eleventy.executeBuild (/Users/[redacted]/node_modules/@11ty/eleventy/src/Eleventy.js:1175:7)

So then I tried redefining all of the default spriteConfig options, which worked:

config.addPlugin(SVGSpritePlugin, {
  path: './src/assets/icons/sprite',
  spriteConfig: {
    mode: {
      inline: true,
      symbol: {
        sprite: 'sprite.svg',
        example: false,
      },
    },
    shape: {
      transform: [{
        svgo: {
          plugins: [{
            name: 'preset-default',
            params: {
              overrides: {
                collapseGroups: false,
                namespaceClassnames: false,
              }
            }
          }]
        }
      }],
      id: {
        generator: (_, file) => {
          const path_separator = '--'
          const whitespace_separator = '_'
          const template = 'svg-%s'
          const pathname = file["relative"].split(path.sep).join(path_separator)
          return util.format(template, path.basename(pathname.replace(/\s+/g, whitespace_separator), '.svg'));
        }
      },
    },
    svg: {
      xmlDeclaration: false,
      doctypeDeclaration: false,
    },
  }
});

For some reason, though, all of my class names are being prefixed with d. I can’t seem to figure out how to disable that but luckily I don’t need classes on my SVGs.

Anyway, thanks for building this plugin! Aside from this minor issue it’s been working great on a couple of projects.

ivanahcosta commented 1 year ago

I guess the class names gets prefixed with a letter (in my case, it's an "a") because of svg-sprite svg.namespaceClassnames (https://github.com/svg-sprite/svg-sprite/blob/main/docs/configuration.md#sprite-svg-options) default option:

"In order to avoid CSS class name ambiguities, the default behavior is to namespace CSS class names in the source SVGs before compiling them into a sprite. Each class name is prepended with a unique string."

So whoever needs it just pass this option as false and the class names won't be modified.

As for the default options getting overridden it happens because in line 18 of the .eleventy.js file (https://github.com/patrickxchong/eleventy-plugin-svg-sprite/blob/main/.eleventy.js#L18) the Object.assign() method makes a shallow copy of target (defaultOptions) object so its nested values aren't being copied, then the code breaks because at line 71 of the SVGSprite file https://github.com/patrickxchong/eleventy-plugin-svg-sprite/blob/main/src/SVGSprite.js#L71 since the spriteConfig.mode becomes undefined. I guess it would be interesting to change it with a deep merging logic instead or maybe making it explicit that it's necessary to copy the default options of spriteConfig in case one wants to modify/add new options

tedw commented 1 year ago

@ivanahcosta Thanks for looking into that!

patrickxchong commented 1 year ago

Hello @tedw my humble apologies for the late revert, it's been a busy season. Thanks a lot @ivanahcosta for debugging the source of the issue. I tried a few deep merge methods on the web but they didn't work well so I resorted to using lodash's merge method instead.

Have published the fixed version 2.4.1 on npm and created a working example at https://github.com/patrickxchong/eleventy-plugin-svg-sprite/tree/main/demo/custom_spriteconfig with the use case you shared earlier @tedw.

Hope this helps! Can close this issue if you've tested the new version works as expected.

tedw commented 1 year ago

@patrickxchong No apologies necessary. I appreciate the update, and your work on the plugin! 🙏

patrickxchong commented 1 year ago

You're most welcome. Closing the issue now