svg-sprite / gulp-svg-sprite

SVG sprites & stacks galore — Gulp plugin wrapping around svg-sprite that reads in a bunch of SVG files, optimizes them and creates SVG sprites and CSS resources in various flavours
MIT License
647 stars 43 forks source link

Inconsistent xmlns attributes in each def #70

Closed benjarwar closed 2 years ago

benjarwar commented 7 years ago

I'm compiling a sprite and checking the file into my repo. But I'm getting inconsistent results. Sometimes, the xmlns attribute is added to each embedded <svg>, sometimes it is not. This is causing minor inconvenience, as it causes diffs in our pull requests when there aren't actual changes.

I've referenced what you wrote here about the inline setting and tried adding inline: false to my config, but it doesn't seem to solve the problem.

Here's my pipe:

.pipe(svgSprite({
  mode: {
    defs: {
      dest: '.',
      prefix: '',
      sprite: 'sprite.svg'
    }
  },
  svg: {
    dimensionAttributes: false
  },
}))

Thanks in advance!

jkphl commented 7 years ago

@benjarwar I've got no explanation why it should give you inconsistent results, unfortunately, and I never experienced something similar. Could you please send me the source SVGs you're working with as well as tell me your Node / Gulp versions so I can try to reproduce it?

benjarwar commented 7 years ago

Thanks for the reply, @jkphl. This is for client work, so I don't think I can send you the source SVGs. I'll see if I can replicate the issue with non-proprietary examples.

In the meantime:

Node v6.9.2 Gulp v3.9.1

The full tasks are like so:

const gulp = require('gulp');
const clean = require('gulp-clean');
const svgSprite = require('gulp-svg-sprite');

gulp.task('clean-svg', () => {
  return gulp.src(paths.svg.dest, {read: false})
    .pipe(clean());
});

gulp.task('svg', ['clean-svg'], function () {
  return gulp.src(paths.svg.src)
    .pipe(svgSprite({
      mode: {
        defs: {
          dest: '.',
          inline: false,
          prefix: '',
          sprite: 'sprite.svg'
        }
      },
      svg: {
        dimensionAttributes: false
      },
    }))
    .pipe(gulp.dest(paths.svg.dest));
});
jkphl commented 7 years ago

@benjarwar Thanks! Yeah, some sample SVGs (in the same style as used for the client project) would probably help to reproduce this. :)

benjarwar commented 7 years ago

Can you tell me which is the expected behavior, given my config without an inline value? Should the nested <svg> elements have xmlns attributes, or no?

jkphl commented 7 years ago

@benjarwar Hard to say off the top of my head (and I don't exactly remember), it might depend on the input SVG. In any case, namespace declarations should be spripped when inline: true.

benjarwar commented 7 years ago

Setting inline: true seems to only affect the root level <svg> element. The nested elements still have xmlns attributes (and it's in the nested elements that I've seen inconsistencies).

zslabs commented 7 years ago

Also experiencing this in https://github.com/rhinogram/rhinostyle if you pull-down, install deps, and run gulp icons

zslabs commented 7 years ago

@jkphl ☝️ That repo I linked to is a good example of source SVGs and what's being built. You can find the built icons in https://github.com/rhinogram/rhinostyle/blob/master/build/svg/sprite.svg

afogelberg commented 7 years ago

I'm also experiencing problems with xmlns attribute added to each symbol, if I use version 1.3.7. However if I use the 1.2.14 version, as also material-design-icons use, the problem disappears.

jkphl commented 6 years ago

I didn't dig deeper into this so far, but I think this might be some SVGO plugin and its default behaviour. Maybe have a look at the removeXMLNS, removeEditorsNSData or removeUnusedNS plugin and disable it (see here for documentation).

Kreeg commented 2 years ago

@afogelberg @benjarwar @zslabs hey! Do you still have this problem?

cssfish commented 2 years ago

this line help me:

shape: {
    transform : [{
        svgo: {
            plugins: [{
                removeXMLNS: true, // remove symbols`s xmlns attributes
            }]
        }
    }]
},
benjarwar commented 2 years ago

@Kreeg no, but only because I haven't been using this library since 2017. 😅

Kreeg commented 2 years ago

Ok, feel free to create a new issue. A lot of changes have been done since 2017 :)