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

Fix XML entities in generated JavaScript #553

Merged
merged 3 commits into from
Sep 12, 2021

Conversation

jsamr
Copy link
Contributor

@jsamr jsamr commented Apr 27, 2021

Summary

This patch fixes the issue reported in #516. The fix targets @svgr/hast-util-to-babel-ast. The handler for XML text parsing now creates a string literal which content is entity-decoded via the entities package. The reference for XML entities is available here: https://www.w3.org/TR/REC-xml/#sec-internal-ent
To be fully XML compliant, we would also have to substitute external entities, those declared in the body of the document. However this is a substantial endeavor out of scope of this fix.

Test plan

Two tests have been added in hast-util-to-babel-ast test folder. See capture below:

image

@vercel
Copy link

vercel bot commented Apr 27, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/gregberge/svgr/JBtwUZdGHsrgqZXtoweY8WvsLBM9
✅ Preview: https://svgr-git-fork-jsamr-fix-entities-gregberge.vercel.app

@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #553 (7b8cc96) into main (7e890a9) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #553      +/-   ##
==========================================
+ Coverage   90.89%   90.99%   +0.10%     
==========================================
  Files          31       31              
  Lines        1076     1077       +1     
  Branches      325      323       -2     
==========================================
+ Hits          978      980       +2     
  Misses         92       92              
+ Partials        6        5       -1     
Impacted Files Coverage Δ
packages/hast-util-to-babel-ast/src/handlers.js 85.41% <100.00%> (+2.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e890a9...7b8cc96. Read the comment docs.

@micheljung
Copy link
Contributor

micheljung commented Jun 16, 2021

@gregberge I hope you enjoyed your vacation, and that you can merge and release this fix soon :D

@lex111
Copy link

lex111 commented Jul 12, 2021

friendly ping for @gregberge, we need this fix :)

@Droppers
Copy link

@gregberge, sorry to bother but any ETA when this pull request will be merged and a new version will be published?

Very surprised this little users ran into this problem (or assumed it's SVGO's fault?)

jsamr and others added 3 commits September 12, 2021 08:56
Co-authored-by: Michel Jung <michel.jung89@gmail.com>
Co-authored-by: Michel Jung <michel.jung89@gmail.com>
@gregberge
Copy link
Owner

Sorry for the delay, thanks for this fix!

@gregberge gregberge merged commit b3998eb into gregberge:main Sep 12, 2021
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.

None yet

5 participants