Why we not use the full image url as cache key?

[chriszyyy]

The logic of caching image url is not working as expected
See here the source code of the issue https://github.com/bubkoo/html-to-image/blob/master/src/getBlobFromURL.ts#:~:text=let%20key%20%3D%20url.replace(/%5C%3F.*/%2C%20%27%27)

function getCacheKey(url: string) {
  let key = url.replace(/\?.*/, '')

  // font resourse
  if (/ttf|otf|eot|woff2?/i.test(key)) {
    key = key.replace(/.*\//, '')
  }

  return key
}

If the image use query string to distinguish which image to get, that image all same format image with used the wrong cached one.

For example:

1. https://www.abc.com/images?id=123
2. https://www.abc.com/images?id=124

The second one will render with the first image because of the cache logic

Expected Behavior

We should use the full URL as key to cache.

Current Behavior

It only use the URL with domain and path, not query string.

Possible Solution

Remove let key = url.replace(/\?.*/, '')

Your Environment

• html-to-image: 1.9.0
• OS: Win10
• Browser: chrome

[biiibooo]

👋 @chriszyyy

Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[biiibooo]

Hiya!
This issue has gone quiet. Spooky quiet. 👻
We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.

Thanks for being a part of the Antv community! 💪💯

[al1khavari]

i have the same issue with this condition

[hallowatcher]

This is so underrated. I had to build my own version of this package in order to comment out the caching section.

I would've proposed a merge request but the build is failing for me locally, and the tests are failing. ESlint is also complaining about the hasOwnProperty calls and the length attribute for booleans

[al1khavari]

This is so underrated. I had to build my own version of this package in order to comment out the caching section.

I would've proposed a merge request but the build is failing for me locally, and the tests are failing. ESlint is also complaining about the hasOwnProperty calls and the length attribute for booleans

I had this issues too , but i deciced to fork this package and build my own version also i fixed the eslint errors to build the package without any test be failed

[biiibooo]

Hiya!
This issue has gone quiet. Spooky quiet. 👻
We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.

Thanks for being a part of the Antv community! 💪💯

[sweepline]

I also have this issue.
Can we do something like this in getBlobFromURL @bubkoo ? It would remain backwards compatible then.

export function getBlobFromURL(
  url: string,
  options: Options,
): Promise<Metadata> {
  const cacheKey = options.cacheQueryString ? url : getCacheKey(url)

I currently have to run a patched version to use this as I'm using this with openlayers.

[biiibooo]

Hiya!
This issue has gone quiet. Spooky quiet. 👻
We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not-stale" to keep this issue open! As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.

Thanks for being a part of the Antv community! 💪💯

[biiibooo]

Hey again!
It’s been 60 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it. Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.
Thanks again for being part of the Antv community! 💪💯