-
-
Notifications
You must be signed in to change notification settings - Fork 20
Make XML validation optional #43
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
Conversation
b0c5430 to
ff2ab4b
Compare
ff2ab4b to
377d7e1
Compare
|
Why do we need an option for this? The previous version was fast enough without an option to disable validation. Not clear whether it did validation or not, but it worked fine, so I would prefer to keep it simple if possible. |
The previous implementation actually did perform basic XML validation. I had to explicitly enable that in @file-type/xml to maintain backward compatibility with is-svg. For reference, file-type itself does not apply XML validation. The current implementation, even with validation enabled, is slightly faster than the previous one. In most use cases, I believe users aren’t concerned with basic XML validation—they just want to check whether the input looks like an SVG. For large SVG files, skipping validation can make a significant difference in performance. The first issue raised since the change, was about performance, so apparently there are users who care. Well, at least one user. |
index.d.ts
Outdated
| export type IsSvgOptions = { | ||
| /** | ||
| Enable or disable XML validation, enabled by default | ||
| */ | ||
| xmlValidation?: boolean; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export type IsSvgOptions = { | |
| /** | |
| Enable or disable XML validation, enabled by default | |
| */ | |
| xmlValidation?: boolean; | |
| }; | |
| export type Options = { | |
| /** | |
| Whether to validate the SVG as proper XML. | |
| Turning this off can improve performance. | |
| @default true | |
| */ | |
| validate?: boolean; | |
| }; |
index.js
Outdated
| import {XmlTextDetector} from '@file-type/xml'; | ||
|
|
||
| export default function isSvg(string) { | ||
| export default function isSvg(string, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export default function isSvg(string, options) { | |
| export default function isSvg(string, {validate} = {}) { |
- Rename `xmValidate` to `validate` - Use inline constructor to initialize empty options object - Renamed TypeScript `type` `IsSvgOptions` to `Options`
cd4b960 to
3b9ca9b
Compare
Introduces an optional option, to disable XML validation.
Disabling the XML validation makes SVG recognition a lot faster, at the cost the SVG XML is not validated.
Difference in processing time:
Resulting in about ~12× to ~60× times short processing time.
ToDo:
Code I used to measure performance:
Resolves #42 (with the note that the actual degradation in performance as described in that issue, I could not reproduce, yet this is significantly reduce parsing time)