Add inlineStyles plugin (rewrite of localStyles plugin)

[strarsis]

This pull request adds the inlineStyles plugin for inlining styles,
also correctly handling selector specificity, cascading,
advanced selectors, etc and intelligently cleaning up matched selectors.

Additionally, direct CSS selector support (powered by css-select, csstree and csso) is added to svgo which
is used by this plugin (querySelectorAll(selector), querySelector(selector), matches(selector)).
Furthermore, the CSSStyleDeclaration interface is added to svgo jsAPI for handling element styles.
As latest addition, Element.classList property is added to svgo JSAPI for proper cleaning up class attributes.
As little extra, the last unnecessary semicolon of inline styles declarationList is cleaned up.

[coveralls]

Coverage remained the same at 89.005% when pulling b68892b on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling bf29ceb on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling fe759cb on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling fe759cb on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling 8ef17ee on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling 8ef17ee on strarsis:inlineStyles into 4822a44 on svg:master.

[GreLI]

CSSO also has a parser (but probably with a bit different AST), so it could be reused without adding dependency.

[strarsis]

Coming back from discussion in #447.

Should this plugin now remove all classes that
a) matched some elements and have been inlined and
b) aren't used multiple times among elements?

Or should this go into a separate plugin just for this task,
which means that this plugin only inlines,
but doesn't change the style elements content and element class attributes?

[GreLI]

Shouldn't this class be removed?

[coveralls]

Coverage remained the same at 89.005% when pulling 5bfd498 on strarsis:inlineStyles into 4822a44 on svg:master.

[GreLI]

I think the pugin should remove inlined classes (as well as emptied <style> elements). There is no reason to keep them. However, if a class is used many times, probably it's better to leave it as is without inlining.

If the plugin will allways inline classes, I don't think it's a good idea to make it run by default.

[strarsis]

OK, so I will adjust the plugin to use csso ast instead and modify the css class removal logic.

[coveralls]

Coverage remained the same at 89.005% when pulling 46aeebe on strarsis:inlineStyles into 4822a44 on svg:master.

[coveralls]

Coverage remained the same at 89.005% when pulling 46aeebe on strarsis:inlineStyles into 4822a44 on svg:master.

[strarsis]

@alexjlockwood: A Pull Request for package.json comes to mind
(which triggers the built-in integration test so it catches possible issues before merging) from
time to time? E.g. https://github.com/fb55/css-select does this with its dependencies using greenkeeper.

[GreLI]

SVGO v1.0.0 with this change has been released now.

[moltar]

Not working correctly for the following SVG:

https://gist.githubusercontent.com/moltar/d4c09168515d4a539aedc105274481ae/raw/3fa7869356c1666089b1690e8f0887d2d169105f/test.svg

Running with:

svgo --multipass --pretty --enable=inlineStyles test.svg

$> svgo -v
1.0.5

[strarsis]

@moltar: I just tried this input svg with svgo version 1.0.5 and found no issues,
neither with svgo usage nor with any visual differences, see https://gist.github.com/strarsis/2bc9a80bc0ce52dee53d41150a8252c2 .

[moltar]

@strarsis But it had no effect... The fill property did not move from <style> block to the attribute on the path.

From test.min.svg:

<path class="st2" d="M62 40h-7v-3c0-1.1-.9-2-2-2h-6c-1.1 0-2 .9-2 2v3h-7c-1.7 0-3 1.3-3 3v16c0 1.7 1.3 3 3 3h24c1.7 0 3-1.3 3-3V43c0-1.7-1.3-3-3-3zm-15-3h6v3h-6v-3zm-9 5h24c.6 0 1 .4 1 1v5H37v-5c0-.6.4-1 1-1zm24 18H38c-.6 0-1-.4-1-1v-9h26v9c0 .6-.4 1-1 1z"/>

[strarsis]

@moltar: You need to set the onlyMatchedOnce option of inlineStyles plugin to false (default is true)
(as shown in #592 (comment)).
Otherwise the CSS class declarations won't be inlined because it matches more than one element.

[moltar]

@strarsis I see, thank you for pointing that out!

[brutzel]

I followed the discussion here for a while and have been using the plugin a few times already. Great work! But from my point of view the default value should be false for that option. I too had to look up how to set plugin options from the command line for the first time and I think this default value is not very intuitive, I would it expect it to be the default behaviour. I am aware this results in much larger files.

[strarsis]

@brutzel: I agree!
@GreLI, @caub: Would it be an improvement to code the default for onlyMatchedOnce option to true?
IMHO inlining everything (and isolating all the styles) is the more expected behaviour with this plugin.

[moltar]

Yup, agreed. I totally expected it to inline everything, that is why I didn't even look for options as my paradigm of thinking was different. The reason I needed to inline everything, is because svg-sprite-loader seems to be stripping out style tags without asking and styles get lost.

[caub]

you can't always inline everything without loss, example with :hover: https://jsfiddle.net/qwx02hgg/

Else for your question, I don't really know, I didn't try this plugin yet

great work!

[strarsis]

@caub: Correct. Non-inlineable styles aren't inlined anyway by the plugin.
(The prefixIds plugin is suitable for further isolating these kinds of styles.)

With onlyMatchedOnce enabled the plugin also does not
inline style blocks which selectors match more than one element,
which is technically not necessary though.

[GreLI]

@strarsis Did you mean setting it to false? We've been there. I think, it's likely will be opposite to optimization. May be better naming or docs will help.

[alexjlockwood]

I guess it depends whether the flag will result in an optimized file or not. On one hand, not inlining everything can save bytes by keeping common code in the style block. On the other hand, not inlining the style can have a big impact on plugins that will be run later on (i.e. you won't be able to merge paths or collapse groups if the elements declare classes).

[strarsis]

I agree with @alexjlockwood. The other plugins have to rely on the declarations of
style attribute of the elements instead using something like Window.getComputedStyle()
(which could be implemented using the new styles API
(caching/performance considerations are required though)).

There may be even the case that an hypothetical extra plugin can "outline" all the styles in
one of the last processing steps. This could help with compression beyond normal gzip.

[GreLI]

Yeah, I dreaming about that for years, but it's far from the current state. Massive rewriting is needed.

[brutzel]

@alexjlockwood right, I think it depends on what you see as optimization. My use case for this plugin is to prepare SVG files for embedding in HTML source. If you do that, CSS classes on multiple SVG elements override each other if they have the same class names. For example Illustrator always uses the same CSS class names st0, st1 and so on for each file. This plugin solves that problem.
Maybe I had misunderstood the purpose of this plugin, but I think this would be a common use case - although it is not really optimization in the sense of minifying SVG files. But maybe there is a more common use case for this plugin that justifies that default value.

[strarsis]

@GreLI: Hm, could it make sense to rewrite svgo to use an existing
DOM/selector library like jsdom as the fork svgz is trying?

[GreLI]

I think @caub has intended to do something like that.

[caub]

Yes, I may try to start a branch on svgo for it, and we could tag it as @next on npm, for a future major version.

It takes times because most plugins should be rewritten

https://github.com/jsdom/jsdom has improved a lot its SVG support, but getComputedStyle is not working as well as browsers yet https://repl.it/@caub/jsdom-getcomputedstyle, we would need to fix it for svgo

We could make a custom DOM impl for svgo, lighter than jsdom, from the current one on svgo

jsdom basically use http://npm.im/nwmatcher (will replaced by http://npm.im/nwsapi soon) for querySelector(All) and matches

Update

Currently stuck on dperini/nwsapi#14 (comment), once it's working, we can integrate nwsapi in a light custom DOM for svg, add |getComputedStyle like jsdom](https://github.com/jsdom/jsdom/blob/master/lib/jsdom/browser/Window.js#L472) and add presentation attributes

[strarsis]

TODO: css-select now also supports pseudo elements/states (fb55/css-select#139).
This can make two steps in the inlineStyles plugin obsolete:

svgo/plugins/inlineStyles.js

Line 91 in e7b8a6c

var selectorsPseudo = cssTools.filterByPseudos(selectorsMq, opts.usePseudos);