JetBrains / svg-sprite-loader

Webpack loader for creating SVG sprites.
MIT License
2.02k stars 270 forks source link

Chunk for SVG sprite isn't properly created in extract mode #364

Closed TobiDimmel closed 4 years ago

TobiDimmel commented 4 years ago

Do you want to request a feature, report a bug or ask a question? report a bug

What is the current behavior? When I integrate the svg-sprite-loader with the Angular CLI and build my app in production mode, following error is thrown:

angular-svg-sprite-issue\node_modules\@angular-devkit\build-webpack\src\utils.js:17
                id: chunk.id.toString(),
                             ^

TypeError: Cannot read property 'toString' of null

Although the error is thrown in the package @angular-devkit/build-webpack the root cause lies in the plug-in of svg-sprite-loader. In the plug-in a new chunk for the SVG sprite is added to the webpack compilation without setting its id: https://github.com/kisenka/svg-sprite-loader/blob/85f07caed508403ab259b5b13eabc97704e0261b/lib/plugin.js#L165-L179

What is the expected behavior? The build in production mode with Angular CLI should work without throwing the error.

The best way is to create repo with minimal setup to demonstrate a problem (package.json, webpack config and your code). Reproduction repo with instructions and a bit more background to the integration: https://github.com/TobiDimmel/angular-svg-sprite-issue

Please tell us about your environment:

Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, gitter, etc) I wonder why the chunk is created and added in the first place. Wouldn't it be enough to only add the sprite to compilation.assets (how it's already done) and be done with it? A short test showed that commenting out the chunk adding solved the problem for my usecase.

betorobson commented 4 years ago

Hi, did you figure out this? I am facing the same issue.

TobiDimmel commented 4 years ago

I didn't in a proper way. I hope they will fix it until we go into production.

You can comment out this line, which solved the problem for me. But I'm not sure what side effects it may cause.

betorobson commented 4 years ago

Ow, I did something similar adding this at line 168 chunk.id = '';

I also do not know the side effects

KrakenTyio commented 4 years ago

could someone fix this

KrakenTyio commented 4 years ago

I create quick FIX plugin hope help you id and ids[0] should be same, instead chunks.length can be svgChunk.name

new class SVGSpriteExtractFixIdPlugin {
                apply(compiler: Compiler) {
                    compiler.hooks.thisCompilation.tap(this.constructor.name, compilation => {
                        compilation.hooks
                            .optimizeChunkAssets
                            .tap(this.constructor.name, (chunks) => {
                                const svgChunk = chunks.find(chunk => !chunk.id && chunk.files.find(value => /\.svg$/.test(value)));
                                if (svgChunk) {
                                    svgChunk.id = chunks.length;
                                    svgChunk.ids.push(chunks.length);
                                }

                                return chunks;
                            });
                    });
                }
            }
kisenka commented 4 years ago

I've removed chunk creation. @TobiDimmel @betorobson @KrakenTyio could you please try it and check if the reported error occurs?

KrakenTyio commented 4 years ago

looks stable

TobiDimmel commented 4 years ago

@kisenka I tried the fix with my linked reproduction repo and my project at work. It works like a charm. Thank you.

AxelKhaldeev commented 4 years ago

@kisenka I am not experienced programmer, so sorry If I ask a stupid question. Is it correct behaviour that after this fix there is no svgsprite in manifest.json (rails + webpacker)?