From 05a7c394c66711a2ab9a50624290a963f5c61705 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 17 Dec 2024 21:59:40 +0000 Subject: [PATCH] Fix workflows and tests, update docs. --- .github/workflows/docker-publish.yml | 1 + README.md | 221 ++++----------------------- src/setupTests.js | 3 + 3 files changed, 37 insertions(+), 188 deletions(-) diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 528fa792..76622e90 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -26,6 +26,7 @@ jobs: with: username: ${{ secrets.pcicdevops_at_dockerhub_username }} password: ${{ secrets.pcicdevops_at_dockerhub_password }} + dockerfile: docker/Dockerfile repository: pcic/climate-explorer-frontend tag_with_ref: true build_args: REACT_APP_APP_VERSION=${{ env.REACT_APP_APP_VERSION }} diff --git a/README.md b/README.md index 6f8f026b..e3ed3cf9 100644 --- a/README.md +++ b/README.md @@ -22,26 +22,13 @@ Note: Avoid `snap` packages. Experience to date suggests it does not result in s ## Configuration -### Environment variables +### Configuration variables -Main configuration of the Climate Explorer frontend is done via environment variables. +Configuration values are managed via a `config.js` file loaded into the app at runtime. Defaults (used for local +development) can be found in [public/config.js](./public/config.js). Adjust these as needed for your local environment +but the defaults _should_ work. -In a Create React App app, [environment variables are managed carefully](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables). -Therefore, most of the environment variables below begin with `REACT_APP_`, as required by CRA. - -CRA also provides a convenient system for setting default values of environment variables -in various contexts (development, production, etc.). - -Brief summary: - -- `.env`: Global default settings -- `.env.development`: Development-specific settings (`npm start`) -- `.env.production`: Production-specific settings (`npm run build`) - -For more details, see the -[CRA documentation](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables)). - -Environment variables for configuring the app are: +Configuration variables for configuring the app are: `PUBLIC_URL` @@ -49,12 +36,6 @@ Environment variables for configuring the app are: - For production, set this to the URL for CE configured in our proxy server. - **WARNING**: The path component of this value **MUST** match `REACT_APP_CE_BASE_PATH` (see below). -`REACT_APP_CE_BASE_PATH` - -- Base **path** of the URL for the CE frontend app; -- For production, set this to the path component of the URL for CE configured in our proxy server. -- **WARNING**: This **MUST** match the path component of `PUBLIC_URL` (see above). - `REACT_APP_APP_VERSION` - Current version of the app. @@ -102,9 +83,18 @@ Environment variables for configuring the app are: - Path within the `public` folder of the external text file. -`NODE_ENV` +### Environment Variables: + +Build configuration can still be managed via Environment variables however once built these will be unchangable +at runtime. Currently the following are defined: -- [**Automatically set; cannot be overridden manually.**](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables) +`PUBLIC_URL`: Defined as the local url when in development mode. Set to `%REPLACE_PUBLIC_URL%` during production +builds to allow for environmental configuration. See [entrypoint.sh](./docker/entrypoint.sh) for this mechanism. + +`APP_VERSION`: set via the `generate-commitish.sh` script, used to bake the current version into the javascript files. + +`NODE_ENV` [(unchangable)](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables): Set automatically to `production` when running `npm run build`, `development` otherwise. Allows for some +optimisation when executing production builds. ### Variable options @@ -126,13 +116,9 @@ we "rebased" it on CRA. ### Installation -You **must** use a version of `npm` >= 5.5.1. This version of `npm` comes with `node` 9.2.0. -If you are using nvm, then run `nvm use 9.2.0` (or higher; ver 11.13 works fine too). - -(`npm` 5.5.1 / `node` 9.2.0 is known to work; `npm` 3.5.2 / `node` 8.10.0 is known to fail to install certain required dependencies. -Intermediate versions may or may not work.) - -Currently reccomended version is `node` 16.x and `npm` 8.x +This app has been tested against node 22, with its accompanying npm 10.9. These can be set up manually (we reccomend +the use of nvm or the `.devcontainer` specification can set up a container based development environment for you, including +executing an initial npm install for you. With the appropriate versions of `node`/`npm` in use: @@ -161,7 +147,7 @@ For building a production app, see below. npm test ``` -### Linting +### Linting / code style Linting is handled by [Prettier](https://prettier.io/). Prettier can be run directly from the command line or using the two aliased commands from the [package.json](package.json); `npm run lint` and `npm run format`. @@ -180,45 +166,13 @@ The workflows setup in actions will automatically build, tag and publish to our #### Configuration, environment variables, and Docker -It is best practice to configure a web app externally, at run-time, typically using environment variables for any -simple (atomic, e.g., string) configuration values. - -Here we run into a problem introduced by CRA: -CRA injects environment variables only at _build time_, not at run time. -["The environment variables are embedded during the build time. Since Create React App produces a static -HTML/CSS/JS bundle, it can’t possibly read them at runtime."](https://facebook.github.io/create-react-app/docs/adding-custom-environment-variables). - -We containerize our apps with Docker. A natural approach to deploying apps with Docker is to build the app as -part of the image build, and then just serve it from a container. However, because of CRA's build-time injection -of environment variables, this means that such Docker images cannot be configured at run-time, that is, when -a container is run. Only static, build-time environment variables are available to CRA apps inside such images. - -It therefore takes some extra effort to inject run-time environment variables (or configuration generally) into -these Dockerized applications. There are two basic approaches: +Run-time configuration options are managed via a `config.js` file loaded into `index.html` when the app is loaded. +Defaults for these configuration options are kept in [public/config.js](./public/config.js), it should have +sensible defaults that allow local development. After deployment, this file can be replaced to adjust configuration +options to match the deployed environment. Within the container it is stored at `/app/config.js`. -1. Build the app, and inject run-time environment variables, as part of the image run (i.e., the command run - by the image includes building the app, which then has access to environment variables provided via the `docker run` - command). \* This is simple but it means that containers are slow to start up and contain a lot of infrastructure - (Node.js, etc.) needed to build the image. This isn't an issue for us, because we don't start many instances and - we don't do it often. - -2. Fetch the environment variables (or a configuration file) from the server. - - This approach has several variants, which are outlined in this - [CRA issue](https://github.com/facebook/create-react-app/issues/2353). - - The way we load the external text and variable options files falls under into this category. - -A key requirement is to be able to configure at run time the the URL at which the app is deployed. -CRA provides a (build-time) environment variable for this, `PUBLIC_URL`. -(Climate Explorer also, as a legacy from pre-CRA versions, uses an overlapping environment variable -`REACT_APP_CE_BASE_PATH`. See Configuration > Environment variables, above.) - -Option 2 makes setting `PUBLIC_URL` _much_ harder to accomplish, and would require significant changes to the -codebase. - -Option 1 makes setting `PUBLIC_URL` simple and requires almost no change to the codebase; -as noted we don't care about the cost of using such containers. - -Therefore we have chosen option 1. +We no longer use environment variables for runtime configuration in order to avoid the build time associated with it. +They can be used to configure specific builds. #### Routing and base path @@ -246,9 +200,6 @@ Instead we must use CRA-provided build-time environment variable `PUBLIC_URL`. to the url you provide (hostname included). This may be particularly useful when using a CDN to host your application. -`PUBLIC_URL` serves much the same purpose as our custom env variable `REACT_APP_CE_BASE_PATH`. -This redundancy will be eliminated in a future verison of CE. - ### Setup using Docker We use Docker for production deployment. @@ -257,92 +208,21 @@ It can also be useful in development; for example, to test a proposed volume mou #### Build docker image manually -Until recently (roughly, Jan 2019), we were using Dockerhub automated builds -to build our images. Dockerhub recently changed their UI and in doing so broke -all the automated builds. For the moment we need to do manual builds. - -Dockerhub images all had the name `pcic/climate-explorer-frontend`. - -To distinguish our manually built images, we are omitting the `pcic/` portion -of the name and just using `climate-explorer-frontend`. - -Build a docker image: - -```bash -docker build -t climate-explorer-frontend \ - --build-arg REACT_APP_APP_VERSION="$(./generate-commitish.sh)" . -``` - -Setting build arg `REACT_APP_APP_VERSION` as above is the most reliable -way to inject an accurate version into the final app. This value can be overridden -when the image is run, but it is not recommended, as it introduces the possibility -of error. - -#### Tag docker image - -Dockerhub automatically assigned the tag `latest` to the latest build. -That was convenient, but ... - -For manual build procedures, -[tagging with `latest` is not considered a good idea](https://medium.com/@mccode/the-misunderstood-docker-tag-latest-af3babfd6375). -It is better (and easy and immediately obvious) to tag with version/release -numbers. In this example, we will tag with version 1.2.3. - -1. Determine the recently built image's ID: - - ```bash - $ docker images - REPOSITORY TAG IMAGE ID CREATED SIZE - climate-explorer-frontend latest 14cb66d3d145 22 seconds ago 867MB - - ``` - -1. Tag the image: - - ```bash - Tag the image - $ docker tag 1040e7f07e5d docker-registry01.pcic.uvic.ca:5000/climate-explorer-frontend:1.2.3 - ``` - -#### Push docker image to PCIC docker registry - -[PCIC maintains its own docker registry](https://pcic.uvic.ca/confluence/pages/viewpage.action?pageId=3506599). We place manual builds in this registry: - -```bash -docker push docker-registry01.pcic.uvic.ca:5000/climate-explorer-frontend:1.2.3 -``` +A Makefile has been included for helping execute the creation of a docker image. This can be executed +via the command `make image`. It will build the javascript code into `build/` and create a docker image. #### Run docker image -As described above, environment variables configure the app. -All are given standard development and production values in the files -`.env`, `.env.development`, and `.env.production`. - -These can be overridden at run time by providing them in the `docker run` command (`-e` option). - -In addition, we mount the configuration files as volumes in the container. -This enables us to update these files without rebuilding or redeploying the app. -See the section below for details. - -Typical production run: - -```bash -docker run --restart=unless-stopped -d - -e PUBLIC_URL= - -e REACT_APP_CE_BASE_PATH= - -e = - -p :8080 - --name climate-explorer-frontend - - v /path/to/external/variable-options.yaml:/app/build/variable-options.yaml - - v /path/to/external/external-texts/default.yaml:/app/build/external-texts/default.yaml - climate-explorer-frontend: -``` +The generated docker image can be tested via the `make up` command. This will start the container based +on the specification defined in the [docker/docker-compose.yaml](./docker/docker-compose.yaml) file. +Configuration options for its execution can be found in its accompanying [config.js](./docker/config.js), +these can be adjusted to suit your preferred configuration (such as adjusting the API source). ## Updating configuration files Certain parts of Climate Explorer are configured in external configuration files. These configuration files are stored in [the `public` folder](https://facebook.github.io/create-react-app/docs/using-the-public-folder). -The path to each configuration file inside this folder specified by an environment variable. +The path to each configuration file inside this folder specified by a value in the `config.js`. Specifically: | Configuration | Env variable | Default value | @@ -360,16 +240,6 @@ in the docker container. (See section above.) This external file can be modified, and the container restarted, to provide an updated version of the variable options file without needing to modify source code, create a new release, or rebuild the image. -To change the configuration file without creating a new release of the app: - -- Update the configuration file in the external file system. -- Restart the container (`docker restart climate-explorer-frontend`) - -Alternatives: - -- Stop the app and start it again with a different value for the associated environment variable, - and a corresponding volume mount for this new file. - To prevent tears, hair loss, and the cursing of your name by future developers (including yourself), we **strongly recommend also updating** the source configuration files in the repo (in the `public` folder) with any changes made, so that they are in fact propagated to later versions. "Hot updates" should not be stored @@ -389,28 +259,3 @@ git commit -m"Bump to version x.x.x" git tag -a -m"x.x.x" x.x.x git push --follow-tags ``` - -## Code style standard compliance - -We have nominally -[adopted](https://github.com/pacificclimate/climate-explorer-frontend/issues/138) -the default ESLint code style. -We aren't enforcing it right now, unfortunately. - -Enforcement aside, we continue to commit code that is in violation of these -standards, which is undesirable for at least two reasons: - -- Following a coding standard makes the code _much_ easier to read. -- For those of us with an IDE plugin that flags standards violations, - they appear _all over the place_, which is distracting to say the least. - -The coding standard we adopted has a lot of rules, but the following are the -ones we are violating most. A small effort could radically reduce the number -of new violations we introduce. In approximate order of frequency of violation: - -1. Limit line length to 80 characters. -1. Use single quotes for strings. (Double quotes are visually noisier.) -1. Place a space after a begin comment delimiter. (`// comment...`, not `//comment...`.) -1. Place a space between `if` and `for` and the opening parenthesis. (`if (cond)`, not `if(cond)`) -1. Declare variables with `const` or `let`, in that order of preference; avoid `var`. (`const` and `let` are scoped.) - - Use `for (const prop in obj)` and `for (const val of iterable)`, but `for (let i = 1; i < n; i++)` diff --git a/src/setupTests.js b/src/setupTests.js index ff4fd397..9fc37e5d 100644 --- a/src/setupTests.js +++ b/src/setupTests.js @@ -1,5 +1,8 @@ import "./core/lodash.mixins"; +// include our default config properties +require("../public/config.js"); + // configure({ adapter: new Adapter() }); // global.SVGPathElement = function () {};