From d6d618ba630aad60eb68be77a3b5ff533b784a4c Mon Sep 17 00:00:00 2001 From: Ross Fenning Date: Mon, 9 Jan 2023 21:01:08 +0000 Subject: [PATCH] Allow hard-coded "sheet" URL to be passed at build time This PR changes a few small things so I'd happily take some feedback and chat about whether they are all a net good for the project. I've done them for myself, but perhaps it improves things for others too. In summary: 1. The webpack build allows env vars (e.g. from Docker build args) to set the sheet ID (URL) and name at build time 2. The code itself is refactored a little to allow dependency-injection of the sheet URL 3. The Dockerfile does the webpack build etc. at build time 4. The Docker build is now multistage which makes it simpler Rationale --------- I liked the idea of being able to run the project as it is while also being able to run a build, even from my own Dockerfile elsewhere, that spits out an HTML file that is pre-baked with the radar data. This would allow deploying it as part of a wider static site, e.g. on Github pages. It seems a reasonable code architecture to hoist out mentions of `window` to the top-level file and inject that info in such that all JS files from `factory.js` are abstracted away from the fact that info came from a query string. Docker Build Changes -------------------- The Docker container running the bulk of the build as a `CMD` on start seemed odd when -- as far as I can see -- the build output is entirely a function of source files alone. Maybe there's more flexibility intended around being able to swap out env vars without having to rebuild the container? I technically do not need the changes for the sheet URL injection, but I'd have to add something to pass the build args into the environment at container runtime. Small Tweaks ------------ Some stuff I noticed along the way: 1. There's two webpack configs but they're _very_ close so I hoisted up the `entry` part to the common file. There may be a reason it is the way it is. 2. I kept finding it odd to have lots of references to "sheet" and "Google Sheet Input" throughout when it's clearly been extended to allow generic JSON and CSV outside of Google sheets. I suspect this is historic but I changed the name of the top export from factory in the hope this is a positive change in the right direction. I can revert if needed though. 3. For JSON and CSV input, the `h1` title just uses the file name. Since we can now inject I've allowed a SHEET_NAME to be set at build time to override that behaviour. Tests ----- I've checked no tests fail, but not added any since the JS changes are _meant_ to be a refactor but perhaps this change warrants an e2e test to verify the new functionality made available. --- Dockerfile | 24 ++++++++++-------------- src/site.js | 13 +++++++++++-- src/util/factory.js | 32 +++++++++++--------------------- webpack.common.js | 6 +++++- webpack.dev.js | 2 -- webpack.prod.js | 2 -- 6 files changed, 37 insertions(+), 42 deletions(-) diff --git a/Dockerfile b/Dockerfile index 54dfb3b00..75b4829c4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,14 +1,7 @@ -FROM nginx:1.23.0 - -RUN apt-get update && apt-get upgrade -y - -RUN curl -fsSL https://deb.nodesource.com/setup_18.x | bash - -RUN apt-get install -y nodejs +FROM node:18 AS build -RUN \ - apt-get install -y \ - libgtk2.0-0 libgtk-3-0 libgbm-dev libnotify-dev libgconf-2-4 libnss3 \ - libxss1 libasound2 libxtst6 xauth xvfb g++ make +ARG SHEET_ID +ARG SHEET_NAME WORKDIR /src/build-your-own-radar COPY package.json ./ @@ -16,7 +9,10 @@ RUN npm install COPY . ./ -# Override parent node image's entrypoint script (/usr/local/bin/docker-entrypoint.sh), -# which tries to run CMD as a node command -ENTRYPOINT [] -CMD ["./build_and_start_nginx.sh"] +RUN npm test && npm run build:prod + +FROM nginx:1.23.0 + +COPY --from=build /src/build-your-own-radar/dist /opt/build-your-own-radar +COPY default.template /etc/nginx/conf.d/default.conf +CMD nginx -g 'daemon off;' diff --git a/src/site.js b/src/site.js index 34d5f31f1..587801c3e 100644 --- a/src/site.js +++ b/src/site.js @@ -3,6 +3,15 @@ require('./images/logo.png') require('./images/radar_legend.png') require('./gtm.js') -const GoogleSheetInput = require('./util/factory') +const RadarInput = require('./util/factory') -GoogleSheetInput().build() +if (process.env.SHEET_ID) { + RadarInput().build(process.env.SHEET_ID, process.env.SHEET_NAME) +} else { + const QueryParams = require('./util/queryParamProcessor') + + var queryString = window.location.href.match(/sheetId(.*)/) + var queryParams = queryString ? QueryParams(queryString[0]) : {} + + RadarInput().build(queryParams.sheetId, queryParams.sheetName) +} diff --git a/src/util/factory.js b/src/util/factory.js index caccc25a0..a09ab9a59 100644 --- a/src/util/factory.js +++ b/src/util/factory.js @@ -162,7 +162,7 @@ const CSVDocument = function (url) { return self } -const JSONFile = function (url) { +const JSONFile = function (url, title) { var self = {} self.build = function () { @@ -180,7 +180,7 @@ const JSONFile = function (url) { contentValidator.verifyContent() contentValidator.verifyHeaders() var blips = _.map(data, new InputSanitizer().sanitize) - plotRadar(FileName(url), blips, 'JSON File', []) + plotRadar(title || FileName(url), blips, 'JSON File', []) } catch (exception) { plotErrorMessage(exception, 'json') } @@ -194,12 +194,6 @@ const JSONFile = function (url) { return self } -const DomainName = function (url) { - var search = /.+:\/\/([^\\/]+)/ - var match = search.exec(decodeURIComponent(url.replace(/\+/g, ' '))) - return match == null ? null : match[1] -} - const FileName = function (url) { var search = /([^\\/]+)$/ var match = search.exec(decodeURIComponent(url.replace(/\+/g, ' '))) @@ -210,23 +204,19 @@ const FileName = function (url) { return url } -const GoogleSheetInput = function () { +const RadarInput = function () { var self = {} var sheet - self.build = function () { - var domainName = DomainName(window.location.search.substring(1)) - var queryString = window.location.href.match(/sheetId(.*)/) - var queryParams = queryString ? QueryParams(queryString[0]) : {} - - if (queryParams.sheetId && queryParams.sheetId.endsWith('.csv')) { - sheet = CSVDocument(queryParams.sheetId) + self.build = function (sheetId, sheetName) { + if (sheetId && sheetId.endsWith('.csv')) { + sheet = CSVDocument(sheetId) sheet.init().build() - } else if (queryParams.sheetId && queryParams.sheetId.endsWith('.json')) { - sheet = JSONFile(queryParams.sheetId) + } else if (sheetId && sheetId.endsWith('.json')) { + sheet = JSONFile(sheetId, sheetName) sheet.init().build() - } else if (domainName && domainName.endsWith('google.com') && queryParams.sheetId) { - sheet = GoogleSheet(queryParams.sheetId, queryParams.sheetName) + } else if (sheetId && sheetName) { + sheet = GoogleSheet(sheetId, sheetName) sheet.init().build() } else { @@ -437,4 +427,4 @@ function plotUnauthorizedErrorMessage() { }) } -module.exports = GoogleSheetInput +module.exports = RadarInput diff --git a/webpack.common.js b/webpack.common.js index dfc68579f..929af8b00 100644 --- a/webpack.common.js +++ b/webpack.common.js @@ -17,6 +17,7 @@ if (env) { } const common = ['./src/common.js'] +const main = ['./src/site.js'] const ASSET_PATH = process.env.ASSET_PATH || '/' @@ -38,13 +39,16 @@ const plugins = [ 'process.env.API_KEY': JSON.stringify(process.env.API_KEY), 'process.env.ENABLE_GOOGLE_AUTH': JSON.stringify(process.env.ENABLE_GOOGLE_AUTH), 'process.env.GTM_ID': JSON.stringify(process.env.GTM_ID), + 'process.env.SHEET_ID': JSON.stringify(process.env.SHEET_ID), + 'process.env.SHEET_NAME': JSON.stringify(process.env.SHEET_NAME), }), ] module.exports = { context: __dirname, entry: { - common: common, + common, + main, }, output: { diff --git a/webpack.dev.js b/webpack.dev.js index 29331b39e..d5441b983 100644 --- a/webpack.dev.js +++ b/webpack.dev.js @@ -2,7 +2,6 @@ const { merge } = require('webpack-merge') const common = require('./webpack.common.js') const path = require('path') const webpack = require('webpack') -const main = ['./src/site.js'] const fs = require('fs') const config = require('./src/config')['development'] const featureTogglesList = Object.keys(config.featureToggles) @@ -14,7 +13,6 @@ fs.writeFileSync(path.join(__dirname, './src/stylesheets/_featuretoggles.scss'), module.exports = merge(common, { mode: 'development', - entry: { main: main }, performance: { hints: false, }, diff --git a/webpack.prod.js b/webpack.prod.js index cfdeeb031..06936af35 100644 --- a/webpack.prod.js +++ b/webpack.prod.js @@ -2,7 +2,6 @@ const { merge } = require('webpack-merge') const common = require('./webpack.common.js') const path = require('path') const webpack = require('webpack') -const main = ['./src/site.js'] const fs = require('fs') const config = require('./src/config')['production'] const featureTogglesList = Object.keys(config.featureToggles) @@ -12,7 +11,6 @@ fs.writeFileSync(path.join(__dirname, './src/stylesheets/_featuretoggles.scss'), module.exports = merge(common, { mode: 'production', - entry: { main }, performance: { hints: false, },