cascornelissen / svg-spritemap-webpack-plugin

SVG spritemap plugin for webpack
MIT License
207 stars 49 forks source link

Should accept sprite.prefix: "" #168

Closed domq closed 3 years ago

domq commented 3 years ago

Description

Putting this in your webpack.config.js triggers an error:

   plugins: [
      // ...
      new SVGSpritemapPlugin("**/*.svg",
        {
          sprite: { prefix: "" }
        })
    ]

Expected behavior

IDs are generated from the file name as usual, except without a prefix

Actual behavior

Error: ValidationError: "sprite.prefix" is not allowed to be empty
    at [...]/node_modules/svg-spritemap-webpack-plugin/lib/options-validator.js:87:19
    at Array.forEach (<anonymous>)
    at module.exports ([...]/node_modules/svg-spritemap-webpack-plugin/lib/options-validator.js:81:13)
    at module.exports ([...]/node_modules/svg-spritemap-webpack-plugin/lib/options-formatter.js:22:5)
    at new SVGSpritemapPlugin ([...]/node_modules/svg-spritemap-webpack-plugin/lib/index.js:29:24)
    at module.exports ([...]/webpack.config.js:129:7)
    at [...]/node_modules/webpack-cli/lib/webpack-cli.js:1565:43
    at Array.map (<anonymous>)
    at evaluateConfig ([...]/node_modules/webpack-cli/lib/webpack-cli.js:1557:24)
    at WebpackCLI.resolveConfig ([...]/node_modules/webpack-cli/lib/webpack-cli.js:1640:47)

System information


  System:
    OS: macOS 11.4
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 3.14 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.2.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 7.13.0 - /usr/local/bin/npm
  npmPackages:
    @babel/core: ^7.14.6 => 7.14.6
    @babel/preset-env: ^7.14.7 => 7.14.7
    @babel/preset-react: ^7.14.5 => 7.14.5
    babel-eslint: ^10.1.0 => 10.1.0
    babel-loader: ^8.2.2 => 8.2.2
    backstopjs: ^5.0.1 => 5.3.4
    bootstrap: ^4.5.2 => 4.6.0
    browser-sync: ^2.27.4 => 2.27.4
    browser-sync-webpack-plugin: ^2.3.0 => 2.3.0
    clipboard: ^2.0.6 => 2.0.8
    colorable: https://github.com/epfl-si/colorable => 1.0.5
    connected-react-router: ^6.9.1 => 6.9.1
    cookieconsent: ^3.1.1 => 3.1.1
    copy-webpack-plugin: ^9.0.1 => 9.0.1
    css-loader: ^5.2.6 => 5.2.6
    eslint: ^7.30.0 => 7.30.0
    eslint-config-airbnb: ^18.2.1 => 18.2.1
    eslint-plugin-import: ^2.23.4 => 2.23.4
    eslint-plugin-jsx-a11y: ^6.4.1 => 6.4.1
    eslint-plugin-react: ^7.24.0 => 7.24.0
    eslint-plugin-react-hooks: ^4.2.0 => 4.2.0
    feather-icons: ^4.28.0 => 4.28.0
    flickity: 2.2.1 => 2.2.1
    flickity-as-nav-for: ^2.0.1 => 2.0.1
    flickity-fullscreen: ^1.1.1 => 1.1.1
    history: ^4.7.2 => 4.10.1
    html-webpack-plugin: ^5.3.2 => 5.3.2
    imagesloaded: ^4.1.4 => 4.1.4
    immer: ^9.0.5 => 9.0.5
    intro.js: ^2.9.3 => 2.9.3
    jquery: ^3.6.0 => 3.6.0
    jquery-mousewheel: ^3.1.13 => 3.1.13
    js-beautify: ^1.14.0 => 1.14.0
    mini-css-extract-plugin: ^2.1.0 => 2.1.0
    multiple-select: ^1.5.2 => 1.5.2
    nanoid: ^3.1.23 => 3.1.23
    node-sass: ^6.0.1 => 6.0.1
    npm-run-all: ^4.1.5 => 4.1.5
    object-fit-images: ^3.2.4 => 3.2.4
    pickadate: ^3.6.4 => 3.6.4
    popper.js: ^1.16.1 => 1.16.1
    postcss: ^8.2.15 => 8.3.5
    postcss-loader: ^6.1.1 => 6.1.1
    postcss-preset-env: ^6.7.0 => 6.7.0
    rc-tooltip: ^5.1.1 => 5.1.1
    react: ^17.0.2 => 17.0.2
    react-copy-to-clipboard: ^5.0.3 => 5.0.3
    react-css-collapse: ^4.1.0 => 4.1.0
    react-dom: ^17.0.2 => 17.0.2
    react-is: ^17.0.2 => 17.0.2
    react-markdown: ^6.0.2 => 6.0.2
    react-redux: ^7.2.4 => 7.2.4
    react-router: ^5.2.0 => 5.2.0
    react-router-dom: ^5.2.0 => 5.2.0
    react-syntax-highlighter: ^15.4.3 => 15.4.3
    redux: ^4.1.0 => 4.1.0
    redux-devtools-extension: ^2.13.9 => 2.13.9
    redux-thunk: ^2.3.0 => 2.3.0
    sass-loader: ^12.1.0 => 12.1.0
    selectize.js: ^0.12.12 => 0.12.12
    style-loader: ^3.0.0 => 3.0.0
    styled-components: ^5.3.0 => 5.3.0
    stylelint: ^13.13.1 => 13.13.1
    stylelint-config-recommended-scss: ^4.2.0 => 4.2.0
    stylelint-scss: ^3.19.0 => 3.19.0
    svg-spritemap-webpack-plugin: ^4.0.3 => 4.0.3
    tablesaw: ^3.1.2 => 3.1.2
    twig: ^1.15.4 => 1.15.4
    walk-sync: ^3.0.0 => 3.0.0
    webpack: ^5.42.1 => 5.42.1
    webpack-cli: ^4.7.2 => 4.7.2
    webpack-merge-and-include-globally: ^2.3.4 => 2.3.4
    yaml: ^1.10.2 => 1.10.2
    yaml-loader: ^0.6.0 => 0.6.0
domq commented 3 years ago

Knowing that svg-spritemap-webpack-plugin eats up spaces in IDs (on account of them being forbidden by the HTML spec), here is a workaround ☺

   plugins: [
      // ...
      new SVGSpritemapPlugin("**/*.svg",
        {
          sprite: { prefix: " " }
        })
    ]

(i.e. a single space for sprite.prefix)

cascornelissen commented 3 years ago

It's not an "error" in the sense that something is going wrong, it's a validation error as in it's not allowed to provide an empty string.

If you don't want a prefix specify false as explained in the docs. If you want something else please explain a bit further 😄

domq commented 3 years ago

I suppose that my request is that the empty string be accepted as a valid prefix? To me at least it does seems valid, and is more intuitive than false which — Unless my ⌘F prefix skills are failing me all of a sudden — I find myself having a hard time concurring that it is explained in the docs, as opposed to glossed over in a type description.

With best regards, Dominique

cascornelissen commented 3 years ago

Not sure I agree with the reasoning but it can't hurt to also support an empty string. Feel free to send in a PR 👍🏼

domq commented 3 years ago

Thanks 👍🏼 See https://github.com/cascornelissen/svg-spritemap-webpack-plugin/pull/169

cascornelissen commented 3 years ago

This has been released in version 4.1.0, thanks for your contribution 🚀