Skip to content
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

Reinstated cacheBuster to new-client #1324

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

jesade-vbg
Copy link
Contributor

Closes #1323

@jesade-vbg jesade-vbg added this to the 3.12 milestone Apr 14, 2023
Comment on lines 62 to 69
getMetaValue(key) {
const el = document.getElementsByName(key);
if (el && el[0]) {
return el[0]?.attributes?.content?.value;
}
return null;
}

Copy link
Member

@jacobwod jacobwod Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really necessary, wouldn't something like this be more sufficient and work on all modern browsers anyway?

document.querySelector("meta[name='hajk-client-use-cache-buster']").getAttribute("content");

That oneliner on line 43 would be enough.

//this.useCacheBuster = process?.env?.REACT_APP_USE_CACHE_BUSTER === "true" || false;
this.useCacheBuster = false;
// Lets get the values from generated meta-tags
this.hash = this.getMetaValue("hajk-client-git-hash") || "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be sure that the meta tag will always be called hajk-client-use-cache-buster? I mean, it's created during build time by parsing %REACT_APP_NAME%-use-cache-buster, so another value on REACT_APP_NAME will break this completely.

@jesade-vbg
Copy link
Contributor Author

You're absolutely right regarding these 😂 I should never make PR:s on Friday afternoons 😂 I'll fix on Monday.

@jesade-vbg
Copy link
Contributor Author

Now it's corrected. Thanx

@jesade-vbg jesade-vbg linked an issue Apr 14, 2023 that may be closed by this pull request
@jacobwod jacobwod self-requested a review April 17, 2023 07:06
jacobwod
jacobwod previously approved these changes Apr 17, 2023
Copy link
Member

@jacobwod jacobwod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍

@jesade-vbg jesade-vbg merged commit 4570f52 into develop Apr 17, 2023
@jesade-vbg jesade-vbg deleted the feature/1323-hfetch-cacheBuster branch April 17, 2023 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hfetch - reinstate the cacheBuster functionality.
2 participants