Feature/website cache busting by MichielPater · Pull Request #813

Feature/website cache busting by MichielPater · Pull Request #813 · devicons/devicon

Added cache busting, so that cache will be cleared on client side whenever devicon.json or SVG files are updated.

…ing '?version' to these filenames

Choose a reason for hiding this comment

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

Michiel and I have discussed this with each other and the changes look good. I also tested the URL to fetch devicon.json manually and the result is still the same.

After all, the ? adds query params but since the API don't accept params for this, nothing happens.

Choose a reason for hiding this comment

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

For me, it looks good. I'll better let @amacado check if this PR's fine, though. Thank you for your contribution! 😄👍

		<script src="//ajax.googleapis.com/ajax/libs/angularjs/1.2.19/angular-sanitize.min.js"></script>
		<script src="//ajax.googleapis.com/ajax/libs/angularjs/1.2.19/angular-animate.min.js"></script>
		<script src="assets/js/script.js"></script>
		<script src="assets/js/script.js?2.13.0"></script>

Choose a reason for hiding this comment

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

Having a static value of ?2.13.0 will probably not achieve the required result, or?

Choose a reason for hiding this comment

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

That's only for the script file, to make it update with the cache busting feature. From this point, the script only needs to be cache busted whenever that file changes, but wouldn't as long as it stays the same. The cache busting is done within the file.

Choose a reason for hiding this comment

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

If the file is in angular (which it is), we can also just use variables defined in $scope. That way we never have update the version manually whenever the script.js changes. While we don't do it now, perhaps later down the road we'll need to update the website.

Choose a reason for hiding this comment

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

That is true. However, I don't think we can use the variables we created within the script file itself. But we could use others, or move the definition outside of the file.

Or maybe there is even caching set outside of these files, like in a configuration or the HTML file. The PR was intended to be a small change though.

Choose a reason for hiding this comment

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

Perhaps we can embed angular code in? Since angular files were listed before the script.js, perhaps we can do something like:

<script src='script.js?{{ (new Date()).getTime() }}' />

I'm not 100% sure this is feasible with angular but I'd think it's possible. As for caching setting for the file itself, we'll have to check the GitHub pages doc

Choose a reason for hiding this comment

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

We could replace this <script> tag with the following?

<script>
	document.write("<script src='assets/js/script.js?" + Date.now() + "'><\/script>");
</script>

Just tested @MichielPater's solution for cache-busting JS and it works well

The only thing that's needed is putting the script tag for the document.write in its separate `<script>. If you can add the changes as you suggested, I think we can merge the PR

Choose a reason for hiding this comment

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

This looks good! Thanks for the help 😄 @MichielPater

This was referenced

Sep 7, 2021

GCHQDeveloper926 pushed a commit to GCHQDeveloper926/devicon that referenced this pull request

Dec 20, 2024

* Feature: Cache busting website data (JavaScript and SVG files) by adding '?version' to these filenames

* Updated script.js to use CDN versioning style

* Cache busting script.js as well, to load cache busting changes

* Website cache busting fix

* Cache busting script.js using Date.now()

Co-authored-by: Thomas Bui <43018778+Thomas-Boi@users.noreply.github.com>