Skip to content
This repository was archived by the owner on Dec 22, 2025. It is now read-only.

Replace the paths of src prop with static import#21

Draft
rassvet-ii wants to merge 1 commit intoedwardez:developfrom
rassvet-ii:improve-cache-control
Draft

Replace the paths of src prop with static import#21
rassvet-ii wants to merge 1 commit intoedwardez:developfrom
rassvet-ii:improve-cache-control

Conversation

@rassvet-ii
Copy link
Contributor

As mentioned in #17, this pr replaces image paths with image imports. Imported images are included in the build result and appended a build-specific hash. The Image component requests this hash-appended path and since the hash is build-specific, the responce is able to have long max-age and the immutable directive.
However, since EquipmentImageList exports large objects, the size of / increases by 25 kB as below.

Route (pages)                              Size     First Load JS
┌ ● /                                      59 kB           268 kB
├   └ css/e7f5fb447484bdb6.css             1.27 kB
├   /_app                                  0 B             209 kB
├ ○ /404                                   438 B           209 kB
├ ● /500                                   438 B           209 kB
├ ● /about                                 707 B           210 kB
└ ● /privacy                               1.96 kB         211 kB
// ...

Route (pages)                              Size     First Load JS
┌ ● /                                      83.9 kB         293 kB
├   └ css/e7f5fb447484bdb6.css             1.27 kB
├   /_app                                  0 B             209 kB
├ ○ /404                                   438 B           209 kB
├ ● /500                                   438 B           209 kB
├ ● /about                                 707 B           210 kB
└ ● /privacy                               1.96 kB         211 kB
// ...

@vercel
Copy link

vercel bot commented Nov 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sensei-helper ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2024 0:29am

@edwardez
Copy link
Owner

Hi,

Few concerns on this change:

  1. Are all the files cached? I can see a lot of images still have a short ttl instead of a maximized TTL
    1

  2. For EquipmentImageList.ts, setting aside the size concern, it seems like going forward, every time we add new equipment resources, we have to update this ts file as well? If so, this doesn't sound like a future-proof solution to me.

  3. I think the best way moving forward is to move static files to another CDN and load images from there(no need to move the entire applcation, just images) - github pages might be a good candidate considering all the images are already in this repo. But I'm not sure if github CDN is as stable as vercel CDN. We have a large user base in China, so this might be a concerning point.

  4. That being said, for now I feel like we can just keep the configuration as-is, if it works then just keep it working, wdyt?

@rassvet-ii
Copy link
Contributor Author

  1. That isn't TTL. That Age means how long the image was in a proxy cache and not time to live.
    TTL is max-age in the Cache-Control field. Age and max-age are explained here.
    I checked the preview and the field is public,max-age=31536000,immutable.
  2. Indeed. It seems ugly. However writing generator script feels overengineering. Rather using the header() function and images prop in the next.config.js may be a good solution?
  3. If you are going to do so, Example Loader Configuration may be helpful.
  4. It will work as it's working now, but also it will increase the Edge Requests in the same way. But I have no good ideas. Until we find the one, we might as well leave things as they're working.

Now I have two more solutions. One is a compromise, setting a ≈1 month length max-age to /public files using headers(). This way prevents revalidation requests, but while the cache is "fresh", any updates to the file are not reflected.
The other is a hacky one, requesting /<buildid>/<path> and transfering it to /public/<path> using rewirte or something. (or webpack??? hacky) This method works almost the same as this PR and no longer requires EquipmentImageList.ts. However, its implementation will be more complicated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants