cascornelissen / svg-spritemap-webpack-plugin

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

Cannot read property 'spritemap': after update to v4.0 #163

Closed dkrnl closed 3 years ago

dkrnl commented 3 years ago

Description After update to v4.0 error: Cannot read property 'spritemap' of undefined

Expected behavior Build without error

Actual behavior

(node:7608) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'spritemap' of undefined
    at SVGSpritemapPlugin.updateFilename (/home/user/works/example/node_modules/svg-spritemap-webpack-plugin/lib/index.js:431:43)
    at /home/user/works/example/node_modules/svg-spritemap-webpack-plugin/lib/index.js:221:22
    at Hook.eval [as call] (eval at create (/home/user/works/example/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:7:1)
    at Hook.CALL_DELEGATE [as _call] (/home/user/works/example/node_modules/webpack/node_modules/tapable/lib/Hook.js:14:14)
    at /home/user/works/example/node_modules/webpack/lib/Compilation.js:2470:28
    at /home/user/works/example/node_modules/webpack/lib/Compilation.js:2670:5
    at /home/user/works/example/node_modules/neo-async/async.js:2818:7
    at done (/home/user/works/example/node_modules/neo-async/async.js:3522:9)
    at /home/user/works/example/node_modules/webpack/lib/Compilation.js:2650:7
    at /home/user/works/example/node_modules/webpack/lib/Compilation.js:2738:32

System information

Minimal reproduction

        new SVGSpritemapPlugin([
            'source/img/svg-sprite/*.svg',
        ], {
            output: {
                filename: 'img/svg-sprite.svg',
                svg4everybody: false,
                svgo: false,
            },
            sprite: {
                prefix: (filename) => {
                    return 'icon-';
                },
                generate: {
                    title: false,
                },
            },
        }),

Additional context

cascornelissen commented 3 years ago

For some reason I can't seem to reproduce this but the stacktrace helped a lot. Can you verify the fix in c46f9eb resolves the issue for you?

You can install this from GitHub directly:

npm install cascornelissen/svg-spritemap-webpack-plugin#master
xavierfoucrier commented 3 years ago

Hi @cascornelissen,

I am working on upgrading my project and face the same issue. Installing from your Github repo compiles fine, but there is no outputted SVG file.

I am using webpack@5.37.0 and webpack-dev-server@3.11.2.

Here is my webpack configuration for SVGSpritemapPlugin:

new SVGSpritemapPlugin('src/asset/vector/**/*.svg', {
  output: {
    filename: 'vector/sprite.svg',
    svgo: {
      floatPrecision: 2,
    },
  },
  sprite: {
    prefix: false,
    generate: {
      title: false,
    },
  },
}),
cascornelissen commented 3 years ago

If anyone can provide a repository that I can use to reproduce the issue, that'd help a lot. For some reason I can't seem to reproduce this on my system...

@xavierfoucrier, using your configuration with version 4 of this plugin throws a bunch of errors because the SVGO configuration isn't supported by SVGO 2, see release notes for additional information.

xavierfoucrier commented 3 years ago

@cascornelissen Yes I saw that the configuration changed, I will upgrade it and let you know. By the way, using only SVGSpritemapPlugin() doesn't work too.

I will investigate and drop a minimal reproductible test repository if needed :wink:

xavierfoucrier commented 3 years ago

@cascornelissen just to be sure, something like this works on your side?

output: {
  filename: 'vector/sprite.svg',
  svgo: {
    plugins: [{
      name: 'cleanupNumericValues',
      params: {
        floatPrecision: 2,
      },
    }],
  },
},
cascornelissen commented 3 years ago

Yes, that works on my side when using the latest commit of master as explained in https://github.com/cascornelissen/svg-spritemap-webpack-plugin/issues/163#issuecomment-838124682. I replace the contents of examples/simple/webpack.config.js with this:

const SVGSpritemapPlugin = require('../..');

module.exports = {
    plugins: [
        new SVGSpritemapPlugin('src/**/*.svg', {
            output: {
                filename: 'vector/sprite.svg',
                svgo: {
                    plugins: [{
                        name: 'cleanupNumericValues',
                        params: {
                            floatPrecision: 2,
                        },
                    }],
                },
            },
        })
    ]
};

Then I do:

cd examples/simple/
rm -rf dist/
../../node_modules/.bin/webpack --mode production

Which succeeds with the following output:

asset vector/sprite.svg 680 bytes [compared for emit] [minimized]
asset main.js 0 bytes [compared for emit] [minimized] (name: main)
./src/index.js 41 bytes [built] [code generated]
spritemap-dummy-module 1 bytes [code generated]
webpack 5.37.0 compiled successfully in 160 ms

And a generated spritemap that seems to be correct:

<svg xmlns="http://www.w3.org/2000/svg"><symbol id="sprite-1" viewBox="0 0 24 24"><path d="M19 19H5V5h14m0-2H5a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2V5a2 2 0 0 0-2-2m-7 14h2V7h-4v2h2"/></symbol><symbol id="sprite-2" viewBox="0 0 24 24"><path d="M15 15h-4v-2h2a2 2 0 0 0 2-2V9a2 2 0 0 0-2-2H9v2h4v2h-2a2 2 0 0 0-2 2v4h6m4 2H5V5h14m0-2H5a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2V5a2 2 0 0 0-2-2z"/></symbol><symbol id="sprite-3" viewBox="0 0 24 24"><path d="M15 15v-1.5a1.5 1.5 0 0 0-1.5-1.5 1.5 1.5 0 0 0 1.5-1.5V9a2 2 0 0 0-2-2H9v2h4v2h-2v2h2v2H9v2h4a2 2 0 0 0 2-2m4 4H5V5h14m0-2H5a2 2 0 0 0-2 2v14a2 2 0 0 0 2 2h14a2 2 0 0 0 2-2V5a2 2 0 0 0-2-2z"/></symbol></svg>
xavierfoucrier commented 3 years ago

@cascornelissen on my side it compiles fine for production, so I guess the problem comes from webpack-dev-server usage with this plugin, that was not the case before... Even with writeToDisk option enabled it doesn't create any SVG sprite file. I will create a reproductible test repository and share here.

Another point, the floatPrecision is not properly applied, even when compilation ends without errors: can you check again please? Some SVG with d="M51.55555555555" ends with d="M51.553", with 3 decimals, like SVGO defaults.

Thanks!

xavierfoucrier commented 3 years ago

@cascornelissen I have just sent you an invite on a private sandbox repository!

Looks like the NPM serve option for webpack-dev-server is the problem. Until now, it was working fine with v3.

Thanks for the feedback :wink:

Diono commented 3 years ago

Hi @cascornelissen I have the same error ONLY in development mode

89% sealing after hashing SVGSpritemapPluginD:\Diono\ousuisje\node_modules\svg-spritemap-webpack-plugin\lib\index.js:431
        const oldFilename = this.filenames[identifier];
                                          ^

TypeError: Cannot read property 'spritemap' of undefined
    at SVGSpritemapPlugin.updateFilename (D:\Diono\ousuisje\node_modules\svg-spritemap-webpack-plugin\lib\index.js:431:43)
    at D:\Diono\ousuisje\node_modules\svg-spritemap-webpack-plugin\lib\index.js:221:22
    at Hook.eval [as call] (eval at create (D:\Diono\ousuisje\node_modules\tapable\lib\HookCodeFactory.js:19:10), <anonymous>:12:1)
    at Hook.CALL_DELEGATE [as _call] (D:\Diono\ousuisje\node_modules\tapable\lib\Hook.js:14:14)
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:2470:28
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:2670:5
    at D:\Diono\ousuisje\node_modules\neo-async\async.js:2818:7
    at done (D:\Diono\ousuisje\node_modules\neo-async\async.js:3522:9)
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:2650:7
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:2738:32
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

maybe because of my output filename config :


const svgoConfig = {
  plugins: [
    { name: 'addAttributesToSVGElement', active: false }, // adds attributes to an outer <svg> element (disabled by default)
    { name: 'addClassesToSVGElement', active: false }, // add classnames to an outer <svg> element (disabled by default)
    { name: 'cleanupAttrs', active: true }, // cleanup attributes from newlines, trailing, and repeating spaces
    { name: 'cleanupEnableBackground', active: true }, // remove or cleanup enable-background attribute when possible
    { name: 'cleanupIDs', active: true }, // remove unused and minify used IDs
    { name: 'cleanupListOfValues', params: { floatPrecision: 2 } }, // round numeric values in attributes that take a list of numbers (like viewBox or enable-background)
    { name: 'cleanupNumericValues', params: { floatPrecision: 2 } }, // round numeric values to the fixed precision, remove default px units
    { name: 'collapseGroups', active: true }, // collapse useless groups
    { name: 'convertColors', active: true }, // convert colors (from rgb() to #rrggbb, from #rrggbb to #rgb)
    { name: 'convertPathData', params: { noSpaceAfterFlags: false } }, // convert Path data to relative or absolute (whichever is shorter), convert one segment to another, trim useless delimiters, smart rounding, and much more
    { name: 'convertShapeToPath', active: true }, // convert some basic shapes to <path>
    { name: 'convertStyleToAttrs', active: true }, // convert styles into attributes
    { name: 'convertTransform', active: true }, // collapse multiple transforms into one, convert matrices to the short aliases, and much more
    { name: 'inlineStyles', active: true }, // move and merge styles from <style> elements to element style attributes
    { name: 'mergePaths', params: { noSpaceAfterFlags: false } }, // merge multiple Paths into one
    { name: 'minifyStyles', active: true }, // minify <style> elements content with CSSO
    { name: 'moveElemsAttrsToGroup', active: true }, // move elements' attributes to their enclosing group
    { name: 'moveGroupAttrsToElems', active: true }, // move some group attributes to the contained elements
    { name: 'prefixIds', active: false }, // prefix IDs and classes with the SVG filename or an arbitrary string
    { name: 'removeAttributesBySelector', active: false }, // removes attributes of elements that match a css selector (disabled by default)
    { name: 'removeAttrs', active: false }, // remove attributes by pattern (disabled by default)
    { name: 'removeComments', active: true }, // remove comments
    { name: 'removeDesc', active: true }, // remove <desc>
    { name: 'removeDimensions', active: true }, // remove width/height attributes if viewBox is present (opposite to removeViewBox, disable it first) (disabled by default)
    { name: 'removeDoctype', active: true }, // remove doctype declaration
    { name: 'removeEditorsNSData', active: true }, // remove editors namespaces, elements, and attributes
    { name: 'removeElementsByAttr', active: false }, // remove arbitrary elements by ID or className (disabled by default)
    { name: 'removeEmptyAttrs', active: true }, // remove empty attributes
    { name: 'removeEmptyContainers', active: true }, // remove empty Container elements
    { name: 'removeEmptyText', active: true }, // remove empty Text elements
    { name: 'removeHiddenElems', active: true }, // remove hidden elements
    { name: 'removeMetadata', active: true }, // remove <metadata>
    { name: 'removeNonInheritableGroupAttrs', active: true }, // remove non-inheritable group's "presentation" attributes
    { name: 'removeOffCanvasPaths', active: false }, // removes elements that are drawn outside of the viewbox (disabled by default)
    { name: 'removeRasterImages', active: true }, // remove raster images (disabled by default)
    { name: 'removeScriptElement', active: true }, // remove <script> elements (disabled by default)
    { name: 'removeStyleElement', active: true }, // remove <style> elements (disabled by default)
    { name: 'removeTitle', active: true }, // remove <title>
    { name: 'removeUnknownsAndDefaults', active: true }, // remove unknown elements content and attributes, remove attrs with default values
    { name: 'removeUnusedNS', active: true }, // remove unused namespaces declaration
    { name: 'removeUselessDefs', active: true }, // remove elements of <defs> without id
    { name: 'removeUselessStrokeAndFill', active: true }, // remove useless stroke and fill attrs
    { name: 'removeViewBox', active: false }, // remove viewBox attribute when possible
    { name: 'removeXMLNS', active: false }, // removes xmlns attribute (for inline svg, disabled by default)
    { name: 'removeXMLProcInst', active: true }, // remove XML processing instructions
    { name: 'reusePaths', active: false }, // Find duplicated elements and replace them with links (disabled by default)
    { name: 'sortAttrs', active: false }, // sort element attributes for epic readability (disabled by default)
  ],
};
...
const svgSpritemapPluginConfig = {
  output: {
    chunk: {
      keep: true,
    },
    svg4everybody: false,
    svg: { sizes: false },
    svgo: svgoConfig,
  },
  sprite: {
    generate: {
      symbol: true,
      title: true,
      use: true,
      view: '-fragment',
    },
    gutter: 0,
  },
  styles: {
    format: 'fragment',
    keepAttributes: true,
  },
};
...
const helper = {
    hashType: '[contenthash]'
};
...
  new SVGSpritemapPlugin(
    path.resolve(__dirname, '../www/src/sprite/extra/*.svg'),
    {
      output: {
        ...svgSpritemapPluginConfig.output,
        filename: `extra.${helper.hashType}.svg`,
      },
      sprite: {
        ...svgSpritemapPluginConfig.sprite,
      },
      styles: {
        ...svgSpritemapPluginConfig.styles,
        // Path to the styles file, note that this method uses the `output.publicPath` webpack option
        // to generate the path/URL to the spritemap itself so you might have to look into that
        filename: path.join(__dirname, '../www/src/scss/common/_extra.scss'),
      },
    }
  ),
Diono commented 3 years ago

other thing => i did change my output.filename but the scss still keep old value => webpack config :

new SVGSpritemapPlugin(...
output: {
    filename: `extra.${Date.now()}`.svg`
}
...)

my generate file scss :

$sprites: (
    'sprite01': "/dist/extra.[contenthash].svg#sprite-sprite01-fragment",
    'sprite02': "/dist/extra.[contenthash].svg#sprite-sprite02-fragment",
)

[contenthash] is not replace or converted... (development mode)

cascornelissen commented 3 years ago

@xavierfoucrier, thanks for the repository, that helped a lot. It's indeed a problem that only pops up in watch mode. It should be fixed in ece1ea4.

These fixes should also resolve the issue with the SVGO plugins not getting applied, using your example repository with floatPrecision: 2, r="35.278888888888" is getting converted to r="35.28".


@Diono, with regards to:


Can both of you try out these changes and report back?

npm install cascornelissen/svg-spritemap-webpack-plugin#master
xavierfoucrier commented 3 years ago

@cascornelissen I will tomorrow! Thanks 💪

xavierfoucrier commented 3 years ago

Hi @cascornelissen,

Yes I can confirm it works fine :+1: Can you publish a new release today?

Thanks a lot!

Diono commented 3 years ago

it's not fine for me :/ i have a new error at the build now (with cascornelissen/svg-spritemap-webpack-plugin#master)

ERROR in ./www/src/scss/home.scss
Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ./node_modules/css-loader/dist/cjs.js):
Error: Can't resolve '/sprite.5df441878dd0ba61a3e4.svg#sprite-separator-light-middle-fragment' in 'D:\Diono\ousuisje\www\src\scss'

actually, the file name is : www/dist/sprite.f0aaf566966dd040d07a.svg its remember me an old bug you fixes before =>https://github.com/cascornelissen/svg-spritemap-webpack-plugin/issues/139

Diono commented 3 years ago

in development mode, its worst now... i have few errors now like this

[webpack-cli] Error: Conflict: Multiple chunks emit assets to the same filename 1620810882601.js (chunks common and errorDocument)
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:3864:12
    at D:\Diono\ousuisje\node_modules\webpack\lib\Cache.js:91:34
    at Array.<anonymous> (D:\Diono\ousuisje\node_modules\webpack\lib\cache\MemoryCachePlugin.js:45:13)
    at D:\Diono\ousuisje\node_modules\webpack\lib\Cache.js:91:19
    at Hook.eval [as callAsync] (eval at create (D:\Diono\ousuisje\node_modules\tapable\lib\HookCodeFactory.js:33:10), <anonymous>:19:1)
    at Cache.get (D:\Diono\ousuisje\node_modules\webpack\lib\Cache.js:75:18)
    at ItemCacheFacade.get (D:\Diono\ousuisje\node_modules\webpack\lib\CacheFacade.js:117:15)
    at D:\Diono\ousuisje\node_modules\webpack\lib\Compilation.js:3810:22
    at arrayEach (D:\Diono\ousuisje\node_modules\neo-async\async.js:2405:9)
    at Object.each (D:\Diono\ousuisje\node_modules\neo-async\async.js:2846:9)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
cascornelissen commented 3 years ago

@Diono, can you share a repository that I can use to reproduce/verify this?

Diono commented 3 years ago

yes i did a small project of my config => https://github.com/Diono/test

cascornelissen commented 3 years ago

@xavierfoucrier, I've just published 4.0.1 with these fixes.


@Diono, thanks, I was hoping for a bit of a smaller repository, it's kind of hard figuring out what's happening when there's tens of additional plugins being loaded, there's CSS files getting generated containing JS, but I took a quick look...

If you can provide a minimal repository that explains this issue I can take another look.

xavierfoucrier commented 3 years ago

@cascornelissen thanks a lot! :wink:

Diono commented 3 years ago

I try to debug my project.

I guess when sass-loader read the generated scss file, the sprite.svg is not yet build in the webpack.output.publicPath. It should be a reason why i have this error Error: Can't resolve '/sprite.1621280929931.svg#sprite-separator-light-middle-fragment' in 'D:\Diono\ousuisje\www\src\scss'

who's the first generated? the sprite svg or the scss file?

Diono commented 3 years ago

OK i found the changes! before the v4, in the generated scss file, the svg path in background-image:url(...) was prefixed with the output.publicPath after the v4, it's no more prefixed...

Diono commented 3 years ago

In my sprite.scss i had background-image: url('/dist/sprite.svg#my-fragment') but now with the v4.0.1 i have background-image: url('/sprite.svg#my-fragment') ... the /dist/ missing

cascornelissen commented 3 years ago

That's the thing, it should still do that:

https://github.com/cascornelissen/svg-spritemap-webpack-plugin/blob/31e7ace01bd682c537438923c7876f703ea878e5/lib/index.js#L284-L292

I was trying to debug why it's not working in your project but there's so many irrelevant things going on in that repository that I can't figure it out easily. While I do enjoy fixing bugs, I don't enjoy spending hours trying to understand some more complicated codebase as much 😅 Which is why I asked for a minimal repository.

You can also debug it yourself, you might get further because you know your codebase. You can just add console.log()s in node_modules/svg-spritemap-webpack-plugin/ (main entry is lib/index.js) to try and see what's going wrong.

Diono commented 3 years ago

yes i understood! I reduce to the minimal config => https://github.com/Diono/test without any options, i still have the problem

cascornelissen commented 3 years ago

Nice, that helps a lot.

The error is thrown because css-loader is trying to resolve the URL to the spritemap at compile time which isn't possible. As shown in the fragments example options.url should be set to false, or a function should be passed that returns false for the spritemap specifically, see the css-loader docs for more information.

That fixes the issue during the compilation but there might still be an issue where the path is incorrect when using the compiled code in the browser. I can't really find a way to verify this using your repository, if you're able to incorporate something like html-webpack-plugin to start a development server when executing npm run dev I can take a look at that too, if needed.

Diono commented 3 years ago

I added html-webpack-plugin : https://github.com/Diono/test If i turn to false the url options in css-loader, the svg url still refer to the root folder (/), not /dist/ (because css-loader not handle this url anymore)

Diono commented 3 years ago

also i have a small warning about division in sass :

DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div(str-length($value), $slice)

More info and automated migrator: https://sass-lang.com/d/slash-div

you should upgrade your generated sass file ;)

cascornelissen commented 3 years ago

Thanks again for the additional information, I'm pretty sure the fix in 7da9664 should resolve the path issue you're running into.

The division symbol deprecation warning should be gone with the changes in 6296251. It's a little more complicated than simply upgrading the generated sass file, other processors such as libsass don't implement math.div ye. Luckily the chunking logic that was there for performance reasons back in the day could be removed.

In the future, creating a separate issue for something totally unrelated is usually the way to go to keep discussions on track ✌🏼

If you could test both of these by installing directly from GitHub, that'd be great! ❤️

npm install cascornelissen/svg-spritemap-webpack-plugin#master
Diono commented 3 years ago

YEAAAAAAHHHHH its works!!! well done! great job :) ❤️

cascornelissen commented 3 years ago

That's great to hear, I've just published 4.0.3 which contains these changes 🚀