imagetools icon indicating copy to clipboard operation
imagetools copied to clipboard

feat: caching of generated images

Open pzerelles opened this issue 2 years ago • 5 comments

  • Quick Checklist
  • [x] I have read the contributing guidelines
  • [x] I have written new tests, as applicable (for bug fixes / features)
  • [x] Docs have been added / updated (for bug fixes / features)
  • [x] I have added a changeset, if applicable
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Feature

  • What is the new behavior (if this is a feature change)? Generated images can be cached to skip transformation of unchanged images.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) No

  • Other information:

pzerelles avatar Nov 29 '23 16:11 pzerelles

🦋 Changeset detected

Latest commit: b0b152f521338558c039f0cc2306bdaedaf27c00

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
vite-imagetools Minor

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

changeset-bot[bot] avatar Nov 29 '23 16:11 changeset-bot[bot]

Codecov Report

Attention: Patch coverage is 83.33333% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 95.47%. Comparing base (dc2f16f) to head (b0b152f).

Files Patch % Lines
packages/vite/src/index.ts 84.37% 10 Missing :warning:
packages/vite/src/utils.ts 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #661      +/-   ##
==========================================
+ Coverage   94.17%   95.47%   +1.30%     
==========================================
  Files          33       33              
  Lines        1235     1283      +48     
  Branches      206      224      +18     
==========================================
+ Hits         1163     1225      +62     
+ Misses         72       58      -14     
Flag Coverage Δ
imagetools-core 95.47% <83.33%> (+1.30%) :arrow_up:
vite-imagetools 95.47% <83.33%> (+1.30%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 29 '23 16:11 codecov[bot]

Are there any other issues with this other than the conflicts that need to be resolved? I have tested it locally and it is working well for me.

AdamMerrifield avatar Jan 04 '24 21:01 AdamMerrifield

I was hoping to get caching support in Vite first, but I don't think that's going to happen soon enough, so I plan to get this PR in. I won't be able to look at it for the next week, but hopefully after that.

benmccann avatar Jan 04 '24 22:01 benmccann

@pzerelles heads up that this PR needs a rebase

benmccann avatar Jan 25 '24 22:01 benmccann

Awesome PR, I was looking for a way to cache my pictures. Pipeline takes 30 minutes because I have many images. Are there any updates on this PR, when will it be merged? Is there anything I can do to help?

Aragur avatar Mar 09 '24 16:03 Aragur

This one has a merge conflict I think from merging your other PR. Can you update it? Thanks!

benmccann avatar Mar 24 '24 22:03 benmccann

Wow I would love to see this merged !

julien-blanchon avatar Mar 26 '24 16:03 julien-blanchon

Is this stable enouth to be use in dev ? I have almost 20min of build time on Vercel (almost 40 image to be optimize by imagetools) ... this is a nightmare for development. I'm really thinking to stop using imagetools because of that ?

julien-blanchon avatar Mar 26 '24 19:03 julien-blanchon

Is this stable enouth to be use in dev ? I have almost 20min of build time on Vercel (almost 40 image to be optimize by imagetools) ... this is a nightmare for development. I'm really thinking to stop using imagetools because of that ?

I've been using it in dev and production for half a year already.

pzerelles avatar Mar 26 '24 19:03 pzerelles

Is this stable enouth to be use in dev ? I have almost 20min of build time on Vercel (almost 40 image to be optimize by imagetools) ... this is a nightmare for development. I'm really thinking to stop using imagetools because of that ?

I've been using it in dev and production for half a year already.

How did you specify pnpm/npm/yarn to fetch this branch ? Any suggestion for Vercel, I suppose it work out the box

julien-blanchon avatar Mar 26 '24 19:03 julien-blanchon

Is this stable enouth to be use in dev ? I have almost 20min of build time on Vercel (almost 40 image to be optimize by imagetools) ... this is a nightmare for development. I'm really thinking to stop using imagetools because of that ?

I've been using it in dev and production for half a year already.

How did you specify pnpm/npm/yarn to fetch this branch ?

Any suggestion for Vercel, I suppose it work out the box

I use a personal build created from my forked repository. But I hope this will be merged really soon. We are working on it.

pzerelles avatar Mar 26 '24 19:03 pzerelles

It looks to me like if a user changes the way in which they want an image to be transformed that those changes won't get picked up because we prefer the cache.

I think we may be able to remove the need for an index.json file if we write a copy of the image file to the cache directory where the filename is the ID returned by generateImageID. Then we could almost use the logic that exists today, but instead of checking the in-memory generatedImages cache we could check the cache directory.

benmccann avatar Mar 26 '24 23:03 benmccann

It looks to me like if a user changes the way in which they want an image to be transformed that those changes won't get picked up because we prefer the cache.

I think we may be able to remove the need for an index.json file if we write a copy of the image file to the cache directory where the filename is the ID returned by generateImageID. Then we could almost use the logic that exists today, but instead of checking the in-memory generatedImages cache we could check the cache directory.

I see your point with the user changes. I need to include the config somewhere...

The main purpose of the index.json file is to provide the metadata without reading the source image, which could be quite large. generateImageID uses the srcURL, config and imageBuffer to generate the cache, but to recreate it we would need to ready the source image, too.

pzerelles avatar Mar 27 '24 07:03 pzerelles

I refactored the whole PR to use the ID from generateImageID. Since user changes will also change that ID, the former problem is solved. I adjusted tests where necessary.

pzerelles avatar Mar 27 '24 11:03 pzerelles

@pzerelles while using this, i have found that it is possible for 0 byte files to be stored in the cache directory if a build is cancelled while generating images.

I sent a PR to resolve this issue https://github.com/pzerelles/imagetools/pull/1

AdamMerrifield avatar Mar 27 '24 21:03 AdamMerrifield

Yayyy, will this be release soon ? How can I update in the meantime ?

julien-blanchon avatar Mar 28 '24 15:03 julien-blanchon

Yes, it will be released before too long. It will be available when https://github.com/JonasKruckenberg/imagetools/pull/707 is merged. You can subscribe to that PR for updates

benmccann avatar Mar 28 '24 16:03 benmccann

Humm I don't know why but it's seems to not work with 7.0, and

imagetools({
			cache: {
				enabled: true,
				dir: './node_modules/.cache/imagetools',
			}
		}),

In the vite.config.ts.

I'm using Sveltekit with a bunch of `?enhanced' imports.

The node_modules/.cache/imagetools is create but remains empty

julien-blanchon avatar Mar 29 '24 00:03 julien-blanchon

You'll need to wait for https://github.com/sveltejs/kit/pull/12055 for @sveltejs/enhanced-img. You shouldn't need to setup image tools.in your Vite config with @sveltejs/enhanced-img

benmccann avatar Mar 29 '24 01:03 benmccann