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.

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

removeXMLSpace plugin: remove xml:space attribute for inline svg usage #1660

Closed
wants to merge 22 commits into from
Closed

removeXMLSpace plugin: remove xml:space attribute for inline svg usage #1660

wants to merge 22 commits into from

Conversation

IngyuTae
Copy link

@IngyuTae IngyuTae commented Mar 6, 2022

xml:space attribute has been deprecated according to MDN.

SVG supports the built-in XML xml:space attribute to handle whitespace characters inside elements. Child elements inside an element may also have an xml:space attribute that overrides the parent's one.

Deprecated: This feature is no longer recommended. Though some browsers might still support it, it may have already been removed from the relevant web standards, may be in the process of being dropped, or may only be kept for compatibility purposes. Avoid using it, and update existing code if possible; see the compatibility table at the bottom of this page to guide your decision. Be aware that this feature may cease to work at any time.

https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/xml:space

@mogrady88
Copy link

Is this something that will be implemented in a near release?

@alvinometric
Copy link

@TrySound Any plans to merge this?

TrySound and others added 18 commits October 23, 2022 11:40
I'm currently profiling my build pipeline and noticed that the
`strongRound` function showed up a couple of times, followed immediately
with a bit of GC cleanup shortly after. We can speed up that function by
avoiding the string casting caused by `Number.prototype.toFixed()`.

In my project's build pipeline with lots of svg icons this saves about
1.4s in total.

<img width="792" alt="js-tools-strongRound"
src="https://user-images.githubusercontent.com/1062408/204393563-666be3e0-e0ee-4608-9b7d-9ea8352bd36b.png">
I'm currently profiling my build setup and noticed that
`stringifyNumber` pops up here and there. Instead of going with the
double regex replace approach, we'll go with the faster approach in
`removeLeadingZero`.

In total the changes in this PR speed up the build by about `0.9s` in my
project.

<img width="798" alt="stringifyNumber"
src="https://user-images.githubusercontent.com/1062408/204154146-ce67d0c9-faf2-40a1-8186-8d34caea74a5.png">
# Conflicts:
#	lib/svgo/config.js
#	plugins/plugins.js
@SethFalco
Copy link
Member

SethFalco commented Jan 19, 2024

We have a dedicated plugin for removing deprecated attributes in main, so if you're removing it because it's deprecated, you can use that instead. It will be part of the next release.

It will be enabled by default, but will only remove safe attributes. Deprecated attributes that may impact rendering will only be removed if the removeAny parameter is passed.

See:

It could be worth having a safe plugin that removes xml:space="preserve" when it's safe to do, for example, when the text/cdata doesn't contain content that would be impacted by the attribute. However, we should be able to favor removeDeprecatedAttrs over a plugin that indiscriminately removes the attribute. Especially when we consider in most cases, including your example, SVGO was already removing it as default is the default value.

@SethFalco SethFalco closed this Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants