Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

✨ Show version number in app #601

Merged
merged 4 commits into from
Mar 24, 2023
Merged

✨ Show version number in app #601

merged 4 commits into from
Mar 24, 2023

Conversation

mark-pitblado
Copy link
Contributor

Resolves #588 - Display version number somewhere on the app

  • Added two lines to the dockerfile that pull the version from the root package.json file and add it to an .env file that next.js can pull from. This took a while to understand, the docker container environmental variables pulls from the root .env but next.js seems to like an .env file in app/apps/web (at least in the way that I was using it).
  • Adjusted footer.tsx to add the version number (in the form vX.X.X so as to be language agnostic) below the RALLLY logo.
  • If the value for the NEXT_PUBLIC_APP_VERSION environment variable is undefined, then the app will look like normal without anything additional showing. This pertains to people running yarn run dev, as then next.js appears to pull from the root .env instead, and so if they do not set this in their .env then it would be undefined.

Overall, I was a little confused with the .env scope based on the README. The sample.env is located in apps/web/ however the README says to copy it using cp sample.env after cd into the rallly directory.

This is my first pull request to a public repo, so open to feedback on how I went about this process and if I have done anything incorrectly.

@vercel
Copy link

vercel bot commented Mar 23, 2023

@arcticFox-git is attempting to deploy a commit to the rallly Team on Vercel.

A member of the Team first needs to authorize it.

@lukevella
Copy link
Owner

I've been having a look at this and I can see it's a bit more complicated than I originally anticipated so don't worry if you need to drop this. I'll be looking into it soon.

@mark-pitblado
Copy link
Contributor Author

Hi @lukevella, yeah I might leave it to you from here, but I did learn a lot from working through this so very grateful for that! If it helps, below is the gist of what worked and what didn't.

If NEXT_PUBLIC_APP_VERSION is set as an environment variable in the root directory, it will successfully display in the app through yarn dev, yarn build etc, but not through docker build. If it is set in the apps/web directory, it will work through docker build but not through yarn.

Feel free to delete this PR at your convenience!

@lukevella lukevella changed the title Adding version number to footer. ✨ Show version number in app Mar 24, 2023
@vercel
Copy link

vercel bot commented Mar 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
web ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 24, 2023 at 3:16PM (UTC)

import * as React from "react";
import process from "process";
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
import process from "process";

Comment on lines 22 to 28
const [appVersion, setAppVersion] = useState("");

useEffect(() => {
if (process.env.NEXT_PUBLIC_APP_VERSION !== undefined) {
setAppVersion('v' + process.env.NEXT_PUBLIC_APP_VERSION || '');
}
}, []);
Copy link
Owner

Choose a reason for hiding this comment

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

It's not necessary to use any hooks for this. You can define appVersion outside of Footer.

const appVersion = process.env.NEXT_PUBLIC_APP_VERSION;

const Footer...

Comment on lines 6 to 7
RUN apt-get update && apt-get install -y jq
RUN echo "NEXT_PUBLIC_APP_VERSION=$(jq -r '.version' package.json)" > apps/web/.env
Copy link
Owner

Choose a reason for hiding this comment

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

We don't need .env in our Dockerfile. Also I think we can use grep to get the version so we don't need to install jq.

Suggested change
RUN apt-get update && apt-get install -y jq
RUN echo "NEXT_PUBLIC_APP_VERSION=$(jq -r '.version' package.json)" > apps/web/.env
ENV NEXT_PUBLIC_APP_VERSION $(cat package.json | grep '"version"' | cut -d'"' -f4)

Also, this probably needs to be in the runner stage further down.

@lukevella
Copy link
Owner

I think I've found a solution for this though it's not quite what I originally had in mind. Rather than trying to set the value of NEXT_PUBLIC_APP_VERSION from within the Dockerfile I am setting this value through a build argument.

Thanks for your help with this @arcticFox-git.

@lukevella lukevella merged commit 4c3d5bf into lukevella:main Mar 24, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display version number somewhere on the app
2 participants