Skip to content

Commit d6d618b

Browse files
author
Ross Fenning
committed
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.
1 parent 21f256c commit d6d618b

File tree

6 files changed

+37
-42
lines changed

6 files changed

+37
-42
lines changed

Dockerfile

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
1-
FROM nginx:1.23.0
2-
3-
RUN apt-get update && apt-get upgrade -y
4-
5-
RUN curl -fsSL https://deb.nodesource.com/setup_18.x | bash -
6-
RUN apt-get install -y nodejs
1+
FROM node:18 AS build
72

8-
RUN \
9-
apt-get install -y \
10-
libgtk2.0-0 libgtk-3-0 libgbm-dev libnotify-dev libgconf-2-4 libnss3 \
11-
libxss1 libasound2 libxtst6 xauth xvfb g++ make
3+
ARG SHEET_ID
4+
ARG SHEET_NAME
125

136
WORKDIR /src/build-your-own-radar
147
COPY package.json ./
158
RUN npm install
169

1710
COPY . ./
1811

19-
# Override parent node image's entrypoint script (/usr/local/bin/docker-entrypoint.sh),
20-
# which tries to run CMD as a node command
21-
ENTRYPOINT []
22-
CMD ["./build_and_start_nginx.sh"]
12+
RUN npm test && npm run build:prod
13+
14+
FROM nginx:1.23.0
15+
16+
COPY --from=build /src/build-your-own-radar/dist /opt/build-your-own-radar
17+
COPY default.template /etc/nginx/conf.d/default.conf
18+
CMD nginx -g 'daemon off;'

src/site.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@ require('./images/logo.png')
33
require('./images/radar_legend.png')
44
require('./gtm.js')
55

6-
const GoogleSheetInput = require('./util/factory')
6+
const RadarInput = require('./util/factory')
77

8-
GoogleSheetInput().build()
8+
if (process.env.SHEET_ID) {
9+
RadarInput().build(process.env.SHEET_ID, process.env.SHEET_NAME)
10+
} else {
11+
const QueryParams = require('./util/queryParamProcessor')
12+
13+
var queryString = window.location.href.match(/sheetId(.*)/)
14+
var queryParams = queryString ? QueryParams(queryString[0]) : {}
15+
16+
RadarInput().build(queryParams.sheetId, queryParams.sheetName)
17+
}

src/util/factory.js

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ const CSVDocument = function (url) {
162162
return self
163163
}
164164

165-
const JSONFile = function (url) {
165+
const JSONFile = function (url, title) {
166166
var self = {}
167167

168168
self.build = function () {
@@ -180,7 +180,7 @@ const JSONFile = function (url) {
180180
contentValidator.verifyContent()
181181
contentValidator.verifyHeaders()
182182
var blips = _.map(data, new InputSanitizer().sanitize)
183-
plotRadar(FileName(url), blips, 'JSON File', [])
183+
plotRadar(title || FileName(url), blips, 'JSON File', [])
184184
} catch (exception) {
185185
plotErrorMessage(exception, 'json')
186186
}
@@ -194,12 +194,6 @@ const JSONFile = function (url) {
194194
return self
195195
}
196196

197-
const DomainName = function (url) {
198-
var search = /.+:\/\/([^\\/]+)/
199-
var match = search.exec(decodeURIComponent(url.replace(/\+/g, ' ')))
200-
return match == null ? null : match[1]
201-
}
202-
203197
const FileName = function (url) {
204198
var search = /([^\\/]+)$/
205199
var match = search.exec(decodeURIComponent(url.replace(/\+/g, ' ')))
@@ -210,23 +204,19 @@ const FileName = function (url) {
210204
return url
211205
}
212206

213-
const GoogleSheetInput = function () {
207+
const RadarInput = function () {
214208
var self = {}
215209
var sheet
216210

217-
self.build = function () {
218-
var domainName = DomainName(window.location.search.substring(1))
219-
var queryString = window.location.href.match(/sheetId(.*)/)
220-
var queryParams = queryString ? QueryParams(queryString[0]) : {}
221-
222-
if (queryParams.sheetId && queryParams.sheetId.endsWith('.csv')) {
223-
sheet = CSVDocument(queryParams.sheetId)
211+
self.build = function (sheetId, sheetName) {
212+
if (sheetId && sheetId.endsWith('.csv')) {
213+
sheet = CSVDocument(sheetId)
224214
sheet.init().build()
225-
} else if (queryParams.sheetId && queryParams.sheetId.endsWith('.json')) {
226-
sheet = JSONFile(queryParams.sheetId)
215+
} else if (sheetId && sheetId.endsWith('.json')) {
216+
sheet = JSONFile(sheetId, sheetName)
227217
sheet.init().build()
228-
} else if (domainName && domainName.endsWith('google.com') && queryParams.sheetId) {
229-
sheet = GoogleSheet(queryParams.sheetId, queryParams.sheetName)
218+
} else if (sheetId && sheetName) {
219+
sheet = GoogleSheet(sheetId, sheetName)
230220

231221
sheet.init().build()
232222
} else {
@@ -437,4 +427,4 @@ function plotUnauthorizedErrorMessage() {
437427
})
438428
}
439429

440-
module.exports = GoogleSheetInput
430+
module.exports = RadarInput

webpack.common.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ if (env) {
1717
}
1818

1919
const common = ['./src/common.js']
20+
const main = ['./src/site.js']
2021

2122
const ASSET_PATH = process.env.ASSET_PATH || '/'
2223

@@ -38,13 +39,16 @@ const plugins = [
3839
'process.env.API_KEY': JSON.stringify(process.env.API_KEY),
3940
'process.env.ENABLE_GOOGLE_AUTH': JSON.stringify(process.env.ENABLE_GOOGLE_AUTH),
4041
'process.env.GTM_ID': JSON.stringify(process.env.GTM_ID),
42+
'process.env.SHEET_ID': JSON.stringify(process.env.SHEET_ID),
43+
'process.env.SHEET_NAME': JSON.stringify(process.env.SHEET_NAME),
4144
}),
4245
]
4346

4447
module.exports = {
4548
context: __dirname,
4649
entry: {
47-
common: common,
50+
common,
51+
main,
4852
},
4953

5054
output: {

webpack.dev.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ const { merge } = require('webpack-merge')
22
const common = require('./webpack.common.js')
33
const path = require('path')
44
const webpack = require('webpack')
5-
const main = ['./src/site.js']
65
const fs = require('fs')
76
const config = require('./src/config')['development']
87
const featureTogglesList = Object.keys(config.featureToggles)
@@ -14,7 +13,6 @@ fs.writeFileSync(path.join(__dirname, './src/stylesheets/_featuretoggles.scss'),
1413

1514
module.exports = merge(common, {
1615
mode: 'development',
17-
entry: { main: main },
1816
performance: {
1917
hints: false,
2018
},

webpack.prod.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ const { merge } = require('webpack-merge')
22
const common = require('./webpack.common.js')
33
const path = require('path')
44
const webpack = require('webpack')
5-
const main = ['./src/site.js']
65
const fs = require('fs')
76
const config = require('./src/config')['production']
87
const featureTogglesList = Object.keys(config.featureToggles)
@@ -12,7 +11,6 @@ fs.writeFileSync(path.join(__dirname, './src/stylesheets/_featuretoggles.scss'),
1211

1312
module.exports = merge(common, {
1413
mode: 'production',
15-
entry: { main },
1614
performance: {
1715
hints: false,
1816
},

0 commit comments

Comments
 (0)