Font Awesome 6 SVG comments cause errors with plugin

airbnb / babel-plugin-inline-react-svg

A babel plugin that optimizes and inlines SVGs for your React Components.

MIT License

473 stars 92 forks source link

Font Awesome 6 SVG comments cause errors with plugin #124

Open adrianosmond opened 10 months ago

adrianosmond commented 10 months ago

An extra ! has been added in the comments in the SVGs in Font Awesome 6, so they change from <!-- Font Awesome Pro 5... to <!--! Font Awesome Pro 6...

I believe this additional ! is causing issues for this plugin.

Steps to reproduce

Edit the comment in test/fixtures/commented.svg in this repository, adding an extra ! between the <!-- and the F which is currently the first non-whitespace character. It doesn't seem to matter if you add more whitespace before the extra !, as long as the ! is the first non-whitespace character.
Run the tests on the repository

Expected behavior

The tests should pass

Actual behavior

They fail

ljharb commented 10 months ago

Unfortunately the commented fixture doesn't run in tests right now, because babel can't parse it, so I'm not sure how we'd verify the fix.

adrianosmond commented 10 months ago

I'm almost definitely misunderstanding something, but isn't it being run on line 208 of test/sanity.js?

I cloned the repo and ran npm run test with both the existing and the FA6 style comment in that fixture and got one result that looked like a success and one that looked like a failure to me.

ljharb commented 10 months ago

ohh right, sorry, it's only svgo: false that's commented out. i'll look into it.

ljharb commented 10 months ago

This seems like it's a babel parse error, so it's something babel would have to fix. Could you file it there?

adrianosmond commented 10 months ago

I think you're wrong there. Babel is erroring, but that's because Babel is being asked to parse an XML comment as JSX and it isn't valid JSX.

I did a little investigation and the issue is coming from the fact that SVGO deliberately doesn't remove these comments by default as they're apparently a convention used for copyright/licensing. https://github.com/svg/svgo/issues/1811

ljharb commented 10 months ago

hm, ok. in that case i'm not sure what the solution is, since this plugin uses babel to parse SVGs.