w0rm/gulp-svgstore

Symbols are empty after gulp-remember has populated it's cache

AndyOGo opened this issue · 7 comments

I'm currently utilizing gulp-cached and gulp-remember to make my build more incremental.
Unfortunately gulp-svgstore is broken in conjunction with gulp-remember.

I tracked the issue down, to the following lines of code:

  1. gulp-svgstore adds a JSDOM cheerio property to the current file, but only if it does not exist already.
    file.cheerio
  2. This SVG document is cached in $svg $svg
  3. At the end the whole contens of the $svgvariable is appended to the current SVG symbol $symbol.
    This step removes all children of $svgand hence also of file.cheerio, which causes empty <symbol> tags in my sprite after gulp-cached and gulp-remember have done their magic (e.g. after a watch has triggered).

Suggested Fix 1

Cleanup file.cheerio reference in stream._transform function:

delete file.cheerio; 

Suggested Fix 2

Just clone the entire svg before appending it to a symbol. This way the cache stays untouched.

Quick fix utilizing gulp-intercept

gulp.src('src/icons/*.svg')
    .pipe($.cached('sprites:svg'))
    .pipe($.svgmin(function (file) {
        var prefix = path.basename(file.relative, path.extname(file.relative));
        return {
            plugins: [{
                cleanupIDs: {
                    prefix: prefix + '-',
                    minify: config.minifySVG
                }
            }]
        }
    }))
    .pipe($.remember('sprites:svg'))
    .pipe($.intercept(function(file) {
        // this is IMPORTANT!!!!
        // gulp-svgstore adds cheerio property to file, but does not clean up
        // hence this breaks gulp-remember, who remembers an empty cheerio instance.
        delete file.cheerio;
        return file;
    }))
    .pipe($.svgstore())
    .pipe(gulp.dest('dest/icons'))
``
w0rm commented

@AndyOGo the cached cheerio object is mentioned here: https://github.com/w0rm/gulp-svgstore#extracting-metadata-from-combined-svg
gulp-cheerio and gulp-svgstore cache it to not parse svg multiple times.

Do you have any other idea for the fix besides this?

@w0rm You are right. I think I missed to communicate the important point here.
I'm not talking about the file.cheerio cache created in stream._flush at line 125.

I'm talking about the incoming files, and their file.cheerio caches.
After all children of DOM node A are appended to another DOM node B. They no longer exist in A, only in B. See DOM Spec and MDN
The cache is absolutly fine, but as soon as the children are removed, the cache loses it's children too. So it makes no sense to have an empty (dirty) cache. Which is exactly what happens at line 111

BTW your link is about the cheerio cache in stream._flush of line 125 not about the stream input caches at lines 38-40

w0rm commented

@AndyOGo I removed the cache completely, I think it adds more harm than benefits. Version 6.0.0 of gulp-svgstore is now published, because it may be a breaking change if someone relied on this feature.

Thanks for the issue!

@w0rm Wau that was fast. Thank you.
Okay, well I think just the cache of each incoming files are proplematic. The cache of the flushed sprite should be fine (fingers crossed).

I just came up with another possible fix. Just clone the svg before appending it to the symbol.
That way the original cache stays unchanged. (Updated above)

w0rm commented

@AndyOGo I realised that if there is a plugin that changes the content and doesn't update the cached cheerio object inbetween gulp-cheerio and gulp-svgstore, then it will break the behavior. The only possible solution would be to calculate the checksum of the content and pass it along with the cache. But it will require a lot of effort from different gulp plugins.

Passing something along with the gulp file object seems to be a bad idea in the gulp ecosystem, I have had a lot of troubles with sourcemaps that are passed the same way.

@w0rm I see, thats true. Then I thing it's best just avoid caching as you did already.