Generated SVG is not added to spritemap chunk

[pciarach]

Hello!

I'm currently upgrading my project to the webpack v5 and I've noticed in another plugin's output that generated SVG file is not added to the spritemap chunk.

I couldn't find any related topics on the internet so I started debugging, and it turned out that the piece of code responsible for that part is never called during my webpack build.

I've downloaded the repo of your plugin and tried to spot the issue. I've written a test that checks if the SVG file is added to spritemap and it seems that it fails on a clean repo as well:

it('Should include spritemap inside generated chunk', (done) => {
    webpack({
        entry: path.resolve(__dirname, './webpack/index.js'),
        plugins: [
            new SVGSpritemapPlugin(path.resolve(__dirname, 'input/svg/single.svg'), {
                output: {
                    filename: 'spritemap.svg',
                    chunk: {
                        name: 'icons',
                    },
                },
                sprite: {
                    prefix: false
                }
            })
        ]
    }, (error, stats) => {
        const iconsChunk = stats.compilation.namedChunks.get('icons');

        expect(iconsChunk).toBeDefined();
        expect(iconsChunk.files.has('icons.js')).toBeTruthy();
        expect(iconsChunk.files.has('spritemap.svg')).toBeTruthy();
        done();
    });
});

If I move part of the code which should be responsible for that outside of compiler.hooks.make.tap subscription it passes but I'm not very familiar with webpack plugin development so I'm not sure if it can break something else... Is that a correct way of fixing it? Or maybe I'm missing something in configuration of plugin / webpack?

---

System information

  System:
    OS: Windows 10 10.0.19044
  Binaries:
    Node: 16.14.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.11 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.5.0 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    @babel/preset-env: ^7.16.4 => 7.16.4
    @xmldom/xmldom: ^0.7.5 => 0.7.5
    babel-jest: ^27.3.1 => 27.3.1
    codecov: ^3.8.3 => 3.8.3
    cross-env: ^7.0.3 => 7.0.3
    del: ^6.0.0 => 6.0.0
    glob: ^7.2.0 => 7.2.0
    jest: ^27.3.1 => 27.3.1
    joi: ^17.4.2 => 17.4.2
    loader-utils: ^3.2.0 => 3.2.0
    lodash: ^4.17.21 => 4.17.21
    mini-svg-data-uri: ^1.4.3 => 1.4.3
    mkdirp: ^1.0.4 => 1.0.4
    svg-element-attributes: ^1.3.1 => 1.3.1
    svg4everybody: ^2.1.9 => 2.1.9
    svgo: ^2.8.0 => 2.8.0
    webpack: ^5.64.1 => 5.64.1
    webpack-cli: ^4.9.1 => 4.9.1
    webpack-merge: ^5.8.0 => 5.8.0
    webpack-sources: ^3.2.2 => 3.2.2

[cascornelissen]

If I move part of the code which should be responsible for that outside

I'm not entirely sure which part of the code you're referring to here? And I'm also not sure what the exact issue is that you're running into 🤔 There's a reason the sections for expected and actual behavior are included in the bug template 😅

[pciarach]

Well, sorry 😅 I thought that the unit test would be sufficient. I was talking about this piece of code:

            // Make sure we add the generated SVG as a file to the spritemap chunk
            compiler.hooks.thisCompilation.tap(plugin, (compilation) => {
                compilation.hooks.processAssets.tap({
                    name: this.constructor.name,
                    stage: webpack.Compilation.PROCESS_ASSETS_STAGE_OPTIMIZE
                }, () => {
                    compilation.chunks.forEach((chunk) => {
                        if ( chunk.name !== outputOptions.chunk.name ) {
                            return;
                        }

                        chunk.files.add(this.filenames.spritemap);
                    });
                });
            });

And when it comes to expected and actual behavior: I'm using assets webpack plugin which outputs all the assets to the JSON file and groups them by chunk name. This is how it outputs this right now (due to SVG file not being added to the the iconsSprite chunk):

Actual

{
  "iconsSprite": {
    "js": "http://localhost:8080/dist/iconsSprite.chunk.js"
  },
  "": {
    "svg": "http://localhost:8080/dist/iconsSprite.dev.svg"
  }
}

And this is how I would expect it to look:

Expected

{
  "iconsSprite": {
    "js": "http://localhost:8080/dist/iconsSprite.chunk.js"
    "svg": "http://localhost:8080/dist/iconsSprite.dev.svg"
  }
}

[cascornelissen]

Gotcha, thanks for the explanation, and no worries! Your last comment and the initial test case don't really seem to match so I'm having a hard time piercing them together but I'm going off the test case for now... Since simply moving this code outside of compiler.hooks.make, as suggested in the original issue, results in quite a few test cases failing, I think we'll have to investigate this a bit further.

My initial thought is that this is related to a long-running "issue" with this plugin and how webpack plugins work in general, this is described in https://github.com/cascornelissen/svg-spritemap-webpack-plugin/issues/157#issuecomment-808131853. The gist of it is that this plugin allows usage of things like [contenthash] in the filename but these hashes are only available after the compilation has reached a certain stage. AFAIK the make hook is where they first are available, so at that point this.updateFilename is called to update the filename. You're likely not experiencing issues after making the suggested change because you're not using hashes in the filename.

Having said that, I'm definitely open to review a PR where all tests, including the one included in this issue pass.

[cascornelissen]

Then again, maybe it was simpler than that. I've made some changes in b4fba19 that result in your test and all other tests succeeding. Could you try it out on your end by installing directly from master by running the following command?

npm install cascornelissen/svg-spritemap-webpack-plugin#master

[pciarach]

Hello, your fix works like a charm ;)

It seems to be indeed connected with the linked issue as I could see the same behavior in our project i.e. incremental build moving SVG file under the correct chunk. But as I said before, your version works correctly in every scenario so I will simply wait for the released package ;)

Thanks a lot!

[cascornelissen]

That's good to hear, let's hope it won't break for anyone else 🤞🏼 I've just published version 4.4.1 which includes this fix.

[martinbroos]

Thanks for this, I was having the same issue when upgrading to webpack 5. For a while I just used the filename without content hash. I did notice that when using version 3.x with webpack 5 it did work. But either way version 4.4.1 fixes it 👍