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

Query parameters in import paths break SVGO's prefixIds plugin #80

Closed
fvsch opened this issue Jul 24, 2023 · 0 comments · Fixed by #82
Closed

Query parameters in import paths break SVGO's prefixIds plugin #80

fvsch opened this issue Jul 24, 2023 · 0 comments · Fixed by #82

Comments

@fvsch
Copy link
Contributor

fvsch commented Jul 24, 2023

Hi there! Thanks for this nice plugin!

First, here's a reproduction for this issue:
https://stackblitz.com/edit/vite-plugin-svgr-query-in-id?file=vite.config.js,src/App.jsx

I'm using vite-plugin-svgr configured to use SVGR only for imports using a ?jsx query parameter. This looks like:

// vite.config.js
export default {
  plugins: [
    svgr({ exportAsDefault: true, include: '**/*.svg?jsx' })
  ]
}

Which allows import SVG files as either URLs or React components:

import imageUrl from './image.svg';
import ImageComponent from './image.svg?jsx';

I'm using this plugin because it handles this pattern by removing the query string from the import id (unlike @svgr/rollup):

const svgCode = await fs.promises.readFile(
id.replace(/\?.*$/, ""),
"utf8"
);

The issue is that the import id is only “cleaned up” that way when reading the file from disk, but not when passed down to SVGR. And SVGR will pass it as image.svg?jsx down to SVGO, which doesn't handle query strings either in its prefixIds plugin:
https://github.com/svg/svgo/blob/73f7002ab614554446e26acf1552c2505448b96f/plugins/prefixIds.js#L13-L20

So id at class attributes and selectors get prefixed with image_svg?jsx instead of, say, image_svg. And that breaks selectors:

<svg>
  <style>
    /* before */
    .st0 { … }
    /* after */
    .image_svg?jsx__st0 { … } 
  </style>
  <path class="image_svg?jsx__st0" d="" />
</svg>

Now, that's svgo's bug, ideally they should clean up those file names further, and/or at least escape them when printing the selector (you can have a ? in a class name, you just need to escape it in selectors).

But could this be handled in this plugin too? It looks like the same logic used to remove the query string from the import id when reading the file could be used here too:

filePath: id,

(There's another usage in the transformWithEsbuild function call, but I think this one may need to keep the original import id?)

I can make a PR for this if that's helpful.

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 a pull request may close this issue.

1 participant