pass useQueryParamsInCacheKey to getCacheKey

[kfiroo]

@bajceta Thanks for the PR! :) Can you provide some motivation behind it? Why is it needed?

[bajceta]

2 images that share the same url, but have different query parameters will have the same filepath even when using useQueryParamsInCacheKey. In our case we resize images serverside, so the base url is the same for all pictures. End result was that a single image was used everywhere :)

[kfiroo]

@bajceta Good catch! Thanks!

NOTE: Looks like some other commits found their way into this PR, was that intentional?

Regarding the original issue, the bug is actually here. We know the url used in path.getImageFilePath is a cacheableUrl which was already stripped of unused query params.

I think the cleanest fix is to add a default value to the useQueryParamsInCacheKey param in generateCacheKey

function generateCacheKey(url, useQueryParamsInCacheKey = true) {
    const parsedUrl = new URL(url, null, true);
    // ...
    const cacheable = filePath + fileName + type + getQueryForCacheKey(parsedUrl, useQueryParamsInCacheKey);
    return SHA1(cacheable) + '.' + type;
}

Let me know if that works for you :)

[bajceta]

Other commits were a mistake, my bad :) I guess the cleanest option then would be to remove useQueryParamsInCacheKey argument from getQueryForCacheKey because as you said it was already stripped in getCachedUrl, so there should be no need, except for a small performance benefit?

What do you think? I will try it out tomorrow.

[kfiroo]

@bajceta I agree, setting the default value to true and removing the param will both work fine. I like the default param option only because it give more flexibility in the future if we want to use this method differently.

[bajceta]

I updated the PR, with the default param. Works for me :)

[kfiroo]

@bajceta Great! :)