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

?url + SVGO? #52

Open
aradalvand opened this issue Dec 9, 2023 · 11 comments
Open

?url + SVGO? #52

aradalvand opened this issue Dec 9, 2023 · 11 comments
Labels
enhancement New feature or request

Comments

@aradalvand
Copy link
Contributor

aradalvand commented Dec 9, 2023

Hey there!
I have a scenario where I need to import an SVG file as a URL (rather than as a Svelte component), but I still want it to go through SVGO.
Unfortunately, using the ?url option seems to cause SVGO to be bypassed. Is there a way around this? And wouldn't it make more sense in general to have all SVGs go through SVGO regardless of how they're imported? That's certainly what I was expecting.

@aradalvand aradalvand changed the title ?url but with SVGO? ?url + SVGO? Dec 9, 2023
@poppa
Copy link
Owner

poppa commented Dec 9, 2023

Hi @aradalvand!

Well, that really makes no sense for this type of plugin. Rememeber, this is not a runtime plugin, but a compile time one.

Just use SVGO itself directly to preprocess your SVG:s 👍

@poppa poppa added question Further information is requested wontfix This will not be worked on labels Dec 9, 2023
@aradalvand
Copy link
Contributor Author

aradalvand commented Dec 9, 2023

Rememeber, this is not a runtime plugin, but a compile time one.

@poppa Not sure how that means this wouldn't make sense?! I'm very much referring to compile-time SVGO processing.

Currently, this goes through SVGO:

<script>
    import someSvg from './some.svg?dataurl';
</script>

But this doesn't:

<script>
    import someSvg from './some.svg?url';
</script>

I don't think it's unreasonable to suggest that this asymmetry could be undesirable.

@poppa
Copy link
Owner

poppa commented Dec 9, 2023

We'll, there's a fundamental difference between the two: The former is compiled into a string in the resulting JS, the latter is read as an URL runtime by the browser.They are not even remotley the same.

So, the former is used compile time, the latter runtime.

What you are asking for is exactly what SVGO already does. Use SVGO in your build process to generate optimized SVGs on disk.

@aradalvand
Copy link
Contributor Author

aradalvand commented Dec 10, 2023

the latter is read as an URL runtime by the browser.

Vite processes it during the build step, so it's not just handed over to the browser as is; couldn't this plugin hook into Vite's build step and ask Vite to give it any SVG files it encounters?

@poppa
Copy link
Owner

poppa commented Dec 10, 2023

That would mean that this plugin would have to overwrite the source SVG on disk, and that is NOT what this plugin should do. Simply run npx svgo path/to/file.svg instead.

@aradalvand
Copy link
Contributor Author

aradalvand commented Dec 10, 2023

... this plugin would have to overwrite the source SVG on disk

That's definitely not what I'm suggesting. There might be something I'm missing here because this seems obvious to me.
In SvelteKit, you run npm run build to generate build artifacts inside some directory, Vite processes all the source files (including SVG source files) and generates "build copies" of them that are, for example, minified, and so on. It does this with the help of plugins, if there are any.

When Vite is generating, say icon.b1946ac9.svg (from the source file icon.svg) during the build process, can't this plugin hook into that and process that SVG file before its build copy is written to disk?

@poppa
Copy link
Owner

poppa commented Dec 10, 2023

I'm more than happy to accept a pull request.

@poppa poppa closed this as completed Dec 16, 2023
@aradalvand
Copy link
Contributor Author

aradalvand commented Dec 16, 2023

@poppa I'm not sure why you closed the issue?!
You eventually acknowledged that this is doable and that you'd be "more than happy to accept a pull request"; no?

@poppa poppa reopened this Dec 17, 2023
@poppa poppa added enhancement New feature or request and removed question Further information is requested wontfix This will not be worked on labels Jan 7, 2024
poppa added a commit that referenced this issue Jan 7, 2024
We now optimize SVGs that are imported as URLs (for usage in `img@src` attributes f ex) before the asset is written to disk in the build step

Duly note that this won't be the case when running in dev mode, since SvelteKit will mount and use the `src` directory as virtual file system, and thusly load the SVGs raw from `src`

This should solve #52
@poppa
Copy link
Owner

poppa commented Jan 8, 2024

Hopefully this should now work in v4.2.0. Try and see if it works as expected 👍

@aradalvand
Copy link
Contributor Author

aradalvand commented Jan 8, 2024

@poppa This does seem to be working in 4.2.0. Thank you!

One small thing though: the hash that Vite appends to the filename seems to be based on the non-SVGO'ed of the SVG; but it should actually be based on the SVGO'ed SVG instead, because currently changing the SVGO configurations (which might result in a different final SVG) doesn't result in a new hash and by extension doesn't allow cache busting to occur. I think this needs to be corrected; but other than that, it's great, thanks again for implementing this.

@aradalvand
Copy link
Contributor Author

aradalvand commented Jan 8, 2024

One more thing: This doesn't seem to work for .css/.scss files processed by Vite that reference SVGs, for example:

.whatever {
    background-image: url('$lib/icons/some-icon.svg?url');
}

I was hoping this would work (as in it would run the linked SVG through SVGO when generating build artifacts). Is this expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants