JonasKruckenberg / imagetools

Load and transform images using a toolbox :toolbox: of custom import directives!
MIT License
946 stars 59 forks source link

fix: exclude assets with ?raw and ?url queries from processing #750

Open didoin opened 2 months ago

didoin commented 2 months ago
changeset-bot[bot] commented 2 months ago

⚠️ No Changeset found

Latest commit: b67d1c77285a6eada7d860416a99d39cc9d6ff85

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

benmccann commented 2 months ago

I wonder if it should be something more like \?(.*&)?(url|raw) in case it is combined with another query parameter. Probably unlikely, but maybe safer?

didoin commented 2 months ago

The official Vite documentation explicitly talks about ?url suffix and ?raw suffix. Static Asset Handling

Also, I think your pattern doesn't allow specifying future query parameters (perhaps for new transformers) that are prefixed with raw or url (?rawable example, I know it's a bad example 😂).

JonasKruckenberg commented 1 month ago

specifying future query parameters (perhaps for new transformers) that are prefixed with raw or url

For what its worth, I think we should not worry about that right now, if a situation arises in the future where there are more special query params like that we can easily push another version then.

didoin commented 1 month ago

If you feel that a more rigorous regular expression would be more appropriate, that's perfectly fine. From my perspective, though, it might not be necessary in this case, since "vite" expects these parameters as a suffix.

As for the "future query parameters" concern, I was also considering the possibility of custom parameters that users might define through the extendTransforms function. It seems like we could avoid potential issues down the line by addressing this now.