Open zm-cttae opened 1 year ago
I'm really steering away from this change right now. Really do NOT want to delve into the very fragile territory that it brings trying to white-list things. I will add a override path for the SVG path to the impl.utils
so you can monkey-patch in something to build upon while we figure things out...
Sounds all right. Happy hols! I imagine the impl.utils
solution will keep all happy.
filterWinningInlineStyles
algorithm that's "good enough", for any interested. Expand if you want to see a small wall of code.Thanks for giving a shot at trying to implement this!
I agree with @IDisposable. I think this algorithm is getting too complex for it to be feasible to maintain as part of the library. Users who desire better SVG size optimizations should override the behavior on their own. We don't want to maintain code that will have to be frequently updated to keep up with the spec.
A comment on the robust-ness on this algorithm: The algorithm looks good so far. As I understand it, it tries removing the property on an SVG clone of the DOM tree, to see if the computed value changes. If the computed value stays the same, the property can be removed. We process children elements before their parents.
I would like to know if you have tested if removing the property with the given name
(in filterWinningInlineStyles) can affect other properties on element
, or affect the property on the children of element
. Because some styles are derived from other styles, e.g. setting display:flex;
on an element changes the styles that are set on children elements, I'm worried this could cause unintended repercussions. It seems to be more risky than copyUserComputedStyleFast
, which was only removing styles that matched the default as well as the parent's inherited value.
Since my last comment there was a gotcha (removing explicit width
and height
caused a content jump in larger screenshots). Not filtering them (as this library has done) fixed that.
Closing this as we can now package up this filter and users can insert it in the module using the options.onclone
function.
I would like to know if you have tested if removing the property with the given name (in
filterWinningInlineStyles
) can affect other properties on element, or affect the property on the children of element.
I don't have data to present, but as I understand it, the filter isn't aggressive or behaviour-dependent enough to produce this footgun. The user agent stylesheets also avoid flex
and grid
.
One day later: realised that CSS properties are qualified like -this
, so now the filter optimises by targeting the properties with more hyphens first (thus leaving more shorthands on the page).
Reopening as I don't want to be the one to maintain an NPM package.
I've updated https://github.com/1904labs/dom-to-image-more/issues/92#issuecomment-1361552316 for options.onclone
compatibility.
dom-inline-style-filter
package:
Use case: description, code
In an ideal world we want to optimize SVG output by removing styles that can be inherited.
This mainly has an impact at >>1K nodes where the output is in the 1MB scale of magnitude.
There's not much of a story here at the moment, but the library did previously lean on
unset
until the #90 regression:https://github.com/1904labs/dom-to-image-more/blob/2c54d7f87c3bb2a9f16223d225f8f1c0c20e3ac6/src/dom-to-image-more.js#L915-L934
There's two strategies I tried to research:
document.styleSheets
while
loop on.parentNode
, and a hardcoded list of inheritable CSS propstoSvg
TreeWalker
to iterate over the nodesSome starter code, but this code snippet:
while (currentNode) { elementList.push(currentNode); currentNode = treeWalker.nextNode(); }
elementList.forEach(function(element) { const inlineStyles = element.style; const computedStyles = getComputedStyle(element); util.asArray(inlineStyles).forEach(function(name) { if (inlineStyles.cssText.includes(name + ': ' + value + ';')) { const value = inlineStyles.getPropertyValue(name); inlineStyles.removeProperty(name); if (value !== computedStyles.getPropertyValue(name)) { inlineStyles[name] = value; } } }); });