-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Allow configuring cache directory for @sveltejs/enhanced-img #14991
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
base: main
Are you sure you want to change the base?
Conversation
…veltejs#12615) vite-imagetools caches processed images by default in `node_modules/.cache/imagetools`. Storing the cache inside `node_modules` can be undesirable in some scenarios - for example, during clean installs (`npm ci`) in CI environments. This commit adds the ability for configuring an alternative cache directory.
🦋 Changeset detectedLatest commit: b96cdf4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
note i am not familiar with the cache impl of vite-imagetools keeping a persistent cache across builds requires meticulous tracking of anything that could have an effect on the cache content. From dependency versions to configuration, code or asset changes. Otherwise stale cached output is going to lead to hard to track errors. So is that cache really safe to store across builds or is it even flushed on buildStart and just there to avoid doing the same work twice during a single build? |
|
@dominikg it is a cache of processed images during build. It's stored in |
|
@dominikg I assume with #14988 SvelteKit completely delegates caching of optimized images to vite-imagetools. As @elliott-with-the-longest-name-on-github pointed out, vite-imagetools stores its cache by default under This PR opens enhanced-img up for configuration of underlying vite-imagetools. |
| import { image_plugin } from './vite-plugin.js'; | ||
|
|
||
| /** | ||
| * @typedef {{cache?: CacheOptions}} EnhancedImageOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should use the types exported by vite-imagetools so that we don't have to update here if they add new options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right - I've originally opted for a custom interface following the discussion in the linked issue. But looking at it now, using vite-imagetools's types makes more sense. I'll update the PR accordingly.
| namedExports: false | ||
| }; | ||
|
|
||
| if (opts.cache) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason to not pass the opts to imagetools directly?
|
Is there any kind of effort to prevent stale cached images from being reused in a future build although some configuration has changed (eg "remove exif metadata")? Regarding CI and caching of node_modules, it really depends on the provider and package manager. For github+pnpm i believe the setup-node action caches pnpms store dir, not node_modules. So ideally this PR comes with documentation on how to use the custom cache directory and set it up in github with the cache action. How do you solve cache restore there ? You'd need a key to pick the right one in case something has changed or again you end up with an entirely stale cache. |
|
Thanks @dominikg for your feedback!
Not from what I've gathered - imagetools uses a hash of the directives and image content as cache key, but does not incorporate the plugin's options, i.e., toggling (Relevant places in imagetools: generateImageID creates a hash of the directives and hashed image contents coming from here, uses the generated id to read / write from / to cache)
Good point. I do not see a straightforward way of coming up with a reliable key without replicating some of imagetools caching logic, which I don't think makes sense. However, putting the problem of caching in CI aside, opening up Do you think it makes sense going forward? If so, I'll update the PR to directly pass through the options to imagetools, using their types. |
This PR allows configuring the cache directory for @sveltejs/enhanced-img.
By default, processed images are cached in
node_modules/.cache/imagetools. When runningnpm ci(e.g., in CI or on deployment), it completely wipesnode_modulesand thus the cache. Placing the cache outside ofnode_modulesalso allows caching processed images between CI runs.Changes:
Add optional config object parameter to
enhancedImages(). The object intentionally mirrors the cache options of imagetools to avoid coupling the two plugins. The parameter can be omitted to be backwards-compatible with any existing config.Behavior:
imagetools's default cache options apply unless explicitly overwritten.
Example:
To place the caching directory to
./my-cache-dir, provide:For a more detailed discussion, see #12615
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits