Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Does not minify svgs that have the <use> tag in them #47

Closed
bencooper222 opened this issue Mar 2, 2018 · 10 comments
Closed

Does not minify svgs that have the <use> tag in them #47

bencooper222 opened this issue Mar 2, 2018 · 10 comments

Comments

@bencooper222
Copy link

Pretty much what it sounds like. I created an example here. You can view the results of this code in three places:

  1. No minification: https://lying-equinox.glitch.me/
  2. Minification with minifySvg set to false: https://lying-equinox.glitch.me/minify_good
  3. Minification with minifySvg set to default (which is true): https://lying-equinox.glitch.me/minify_bad

If you inspect element the minify_bad path, you can see how htmlnano butchers the <use> tag's contents. Not sure if this is intentional behavior or not but it makes dealing with svgs difficult (which is unfortunate because they tend to create a lot of whitespace and such if not minimized).
@bencooper222 bencooper222 mentioned this issue Mar 13, 2018
Closed
@bencooper222
Copy link
Author

The issue seems to be related to SVGO's default settings which seem to be too aggressive in minimizing.

@davidnagli
Copy link

@voischev @Scrum @maltsev We need this feature for @parcel-bundler. It's currently causing issues for our users. 😞

See: parcel-bundler/parcel#929

@Scrum
Copy link
Member

Scrum commented Mar 13, 2018

@bencooper222 @davidnagli Thanks for ping, I'll try to figure this out soon. Let me know if you find a solution ahead of me :)

@Scrum Scrum self-assigned this Mar 13, 2018
@Scrum Scrum added this to the 0.1.7 milestone Mar 13, 2018
@Scrum Scrum added the bug label Mar 13, 2018
@Scrum Scrum changed the title Does not minify svgs that have the <use> tag in them [fix]: Does not minify svgs that have the <use> tag in them Mar 13, 2018
@Scrum
Copy link
Member

Scrum commented Mar 13, 2018

At the moment I recommend using htmlnano with settings for svgo:

minifySvg: {
    plugins: [ 
        { cleanupIDs: false },
        { removeUnknownsAndDefaults: false},
    ]
}

see example

The issue seems to be related to SVGO's default settings which seem to be too aggressive in minimizing.

yep

@Scrum Scrum removed the bug label Mar 13, 2018
@Scrum Scrum mentioned this issue Mar 13, 2018
Closed
@Scrum
Copy link
Member

Scrum commented Mar 13, 2018

related to work svg/svgo#921

@fregante
Copy link

fregante commented Mar 13, 2018 •
edited
Loading

Edit: Resolved in htmlnano 0.1.7, it errors

In my case, the entire <svg> is replaced with undefined if a use tag is in it:

For example

<!DOCTYPE html><svg><use xlink:href="./file.svg"/></svg>

Becomes

<!DOCTYPE html>undefined

Here's a one-line CLI test:

node -e 'require("htmlnano").process(\'<!DOCTYPE html><svg><use xlink:href="./file.svg"/></svg>\').then(result => console.log(result.html));'

<!DOCTYPE html>undefined

@fregante fregante mentioned this issue Mar 13, 2018
Closed
maltsev added a commit that referenced this issue Mar 13, 2018
@maltsev
Test SVG with <use> minification #47
5058397
@maltsev
Copy link
Member

maltsev commented Mar 13, 2018

I updated all dependencies and released version 0.1.7. Now <use> minification works well.

@maltsev
Copy link
Member

maltsev commented Mar 13, 2018

@bfred-it xlink:href is deprecated. Therefore, I guess, SVGO doesn't process it. If you replace it with href it'd work fine, though.

@Scrum Scrum removed this from the 0.1.7 milestone Mar 14, 2018
@Scrum Scrum changed the title [fix]: Does not minify svgs that have the <use> tag in them Does not minify svgs that have the <use> tag in them Mar 14, 2018
@Scrum Scrum removed their assignment Mar 15, 2018
@fregante
Copy link

fregante commented Mar 15, 2018 •
edited
Loading

@maltsev xlink:href should still be processed because the vast majority of SVGs on the web use it. I doubt href has the same compat.

I think my issue here was the lack of xmlns:xlink="http://www.w3.org/1999/xlink"

This works:

<!DOCTYPE html>
<svg xmlns:xlink="http://www.w3.org/1999/xlink">
	<use xlink:href="./file.svg"/>
</svg>

This doesn't, but it should fail either explicitly or by leaving the SVG as-is, not output undefined

<!DOCTYPE html>
<svg>
	<use xlink:href="./file.svg"/>
</svg>

Edit

This seems to work correctly in htmlnano 0.1.7, :) an error is output when xmlns:xlink is missing

@maltsev
Copy link
Member

maltsev commented Mar 15, 2018

I think my issue here was the lack of xmlns:xlink="http://www.w3.org/1999/xlink"

You're right. htmlnano processes such code correctly:

require('htmlnano')
    .process('<!DOCTYPE html><svg xmlns:xlink="http://www.w3.org/1999/xlink"><use xlink:href="./file.svg"/></svg>')
    .then(result => console.log(result.html));

// Output:
// <!DOCTYPE html><svg xmlns:xlink="http://www.w3.org/1999/xlink"><use xlink:href="./file.svg"/></svg>

But it explicitly fails when xmlns:xlink isn't present:

require("htmlnano")
    .process('<!DOCTYPE html><svg><use xlink:href="./file.svg"/></svg>')
    .then(result => console.log(result.html))
    .catch(err => console.error(err));

// Output:
// Error in parsing SVG: Unbound namespace prefix: "xlink"
// Line: 0
// Column: 34
// Char: >

So it seems that that undefined error should be fixed in parcel-bundler/parcel#996.

@maltsev maltsev closed this as completed Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@maltsev @fregante @Scrum @davidnagli @bencooper222