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

client,webserver: Use the app build version as Version in UI. #3096

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dev-warrior777
Copy link
Contributor

@dev-warrior777 dev-warrior777 commented Nov 24, 2024

Issue: #3091

- Use the value from client/app/version.go
- Place at the foot of Settings dialog
- Re-purpose 'BUILD ID' as the string "Version" for translations and find relevant translations

@dev-warrior777
Copy link
Contributor Author

dev-warrior777 commented Nov 24, 2024

master: c298ca2
image

  • Left some of the old commitHash code in commented out for viewing
  • Some golang _tests are failing for webserver

@dev-warrior777 dev-warrior777 marked this pull request as ready for review November 26, 2024 12:45
client/webserver/site/src/js/settings.ts Outdated Show resolved Hide resolved
client/webserver/webserver.go Outdated Show resolved Hide resolved
dev-warrior777 added 5 commits November 29, 2024 14:49
	- Use the value from client/app/version.go
	- Place at the foot of Settings dialog
	- Re-purpose 'BUILD ID' as "Version for
	  translations and find relevant translations
	- Also replaced the deleted Doc.bind 'Add Dex'
client/webserver/locales/de-de.go Outdated Show resolved Hide resolved
@@ -257,7 +257,7 @@
</div>
</div>

<script src="/js/entry.js?v={{commitHash}}"></script>
<script src="/js/entry.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needed in order to avoid the browser returning a previous version of the js that was cached.

Copy link
Contributor Author

@dev-warrior777 dev-warrior777 Dec 1, 2024

Choose a reason for hiding this comment

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

Ok .. I am not that great with js .. but the variable commitHash has been removed. I did test quite a bit after making the above change.

Is this part ?v={{commitHash}} used as like an Id or a marker? Would a one-time randomly generated number do instead perhaps .. like always use ... /js/entry.js?v=1234567 maybe? And similarly for the link in the header: <link href="/css/style.css?v={{commitHash}}" rel="stylesheet"> at the top of the page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And maybe the build itself can now create the entry.js, entry.js.map?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this part ?v={{commitHash}} used as like an Id or a marker?

Yes, without this, if you check out a different version and run the UI, you will see the previous version of the js and css that were cached.

I think you should just leave the commitHash template func and not change this stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now using Webpack Build ID in place of commitHash

	Also:
	- Translation string 'BUILD ID' is no longer used.
	- 'BUILD ID' => 'Version'
	- Translations updated to mean Version - not Build Id.
	  except arabic as I cannot handle the input method to
	  edit arabic as the arrow/BS keys go the wrong way in
	  the editor. And I do not understand how the editor
	  concatenates arabic strings.
	  Meaning left as previously translated: Build Id.
dev-warrior777 added 2 commits December 2, 2024 20:55
	- Emit a webpack build ID when building site and store in webpack-build-id.txt

	- Embed the webpack build ID into bodybuilder as a url query string:-
	  webpackBuildIdQuery fetches latest webpack build from the webpack-build-id.txt
	  file in app site directory and makes it available to append to the main css
	  link and to the main script link in bodybuilder; this should cause no reload
	  of the main css/js files if they are already cached by the browser.
	  If webpackBuildIdFile is not found return a fallback query that will make the
	  browser reload css/js.

	- Some eslint in webpack dir.
@JoeGruffins
Copy link
Member

JoeGruffins commented Dec 6, 2024

You already have the version there in /client/app so I dont think you need to call from the top consumer 3a1cb56

I'm not good with the html and js side so difficult for me to give valuable comments there.

Copy link
Contributor

@martonp martonp left a comment

Choose a reason for hiding this comment

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

I think we should use what's written here for the caching:
https://webpack.js.org/guides/caching/

This can be a separate PR though, as it's independent from updating the version ID.

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.

3 participants