Skip to content

Commit

Permalink
Use async file operations for helpers
Browse files Browse the repository at this point in the history
  • Loading branch information
colinrotherham committed Sep 22, 2022
1 parent 5ae1389 commit efa705e
Show file tree
Hide file tree
Showing 48 changed files with 616 additions and 319 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/diff-change-to-dist.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ jobs:
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
const fs = require('fs').promises
const diff = await fs.readFile(
const { readFile } = require('fs/promises')
const diff = await readFile(
process.env.GITHUB_WORKSPACE + '/dist.diff', 'utf8'
)
Expand Down
32 changes: 15 additions & 17 deletions app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@ const app = express()
const bodyParser = require('body-parser')
const nunjucks = require('nunjucks')
const { marked } = require('marked')
const util = require('util')
const fs = require('fs')
const path = require('path')

const readdir = util.promisify(fs.readdir)

const helperFunctions = require('../lib/helper-functions')
const fileHelper = require('../lib/file-helper')
const configPaths = require('../config/paths.js')
Expand All @@ -23,7 +19,7 @@ const appViews = [
`${configPaths.node_modules}/govuk_template_jinja`
]

module.exports = (options) => {
module.exports = async (options) => {
const nunjucksOptions = options ? options.nunjucks : {}

// Configure nunjucks
Expand All @@ -37,6 +33,13 @@ module.exports = (options) => {
...nunjucksOptions // merge any additional options and overwrite defaults above.
})

// Cache common component listings
const [examples, components, fullPageExamples] = await Promise.all([
fileHelper.getDirectories(configPaths.examples),
fileHelper.getAllComponents(),
fileHelper.getFullPageExamples()
])

// make the function available as a filter for all templates
env.addFilter('componentNameToMacroName', helperFunctions.componentNameToMacroName)
env.addGlobal('markdown', marked)
Expand Down Expand Up @@ -107,10 +110,6 @@ module.exports = (options) => {

// Index page - render the component list template
app.get('/', async function (req, res) {
const components = fileHelper.allComponents
const examples = await readdir(path.resolve(configPaths.examples))
const fullPageExamples = fileHelper.fullPageExamples()

res.render('index', {
componentsDirectory: components,
examplesDirectory: examples,
Expand All @@ -120,25 +119,24 @@ module.exports = (options) => {

// Whenever the route includes a :component parameter, read the component data
// from its YAML file
app.param('component', function (req, res, next, componentName) {
res.locals.componentData = fileHelper.getComponentData(componentName)
app.param('component', async function (req, res, next, componentName) {
res.locals.componentData = await fileHelper.getComponentData(componentName)
next()
})

// All components view
app.get('/components/all', function (req, res, next) {
const components = fileHelper.allComponents

res.locals.componentData = components.map(componentName => {
const componentData = fileHelper.getComponentData(componentName)
app.get('/components/all', async function (req, res, next) {
res.locals.componentData = await Promise.all(components.map(async componentName => {
const componentData = await fileHelper.getComponentData(componentName)
const defaultExample = componentData.examples.find(
example => example.name === 'default'
)
return {
componentName,
examples: [defaultExample]
}
})
}))

res.render('all-components', function (error, html) {
if (error) {
next(error)
Expand Down
7 changes: 5 additions & 2 deletions app/app.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const cheerio = require('cheerio')
const { Agent, fetch, setGlobalDispatcher } = require('undici')

const lib = require('../lib/file-helper')
const { getAllComponents } = require('../lib/file-helper')

const configPaths = require('../config/paths.js')
const PORT = configPaths.ports.test
Expand Down Expand Up @@ -51,10 +51,13 @@ describe(`http://localhost:${PORT}`, () => {
it('should display the list of components', async () => {
const response = await fetchPath('/')
const $ = cheerio.load(await response.text())

const components = await getAllComponents()
const componentsList = $('li a[href^="/components/"]').get()

// Since we have an 'all' component link that renders the default example of all
// components, there will always be one more expected link.
const expectedComponentLinks = lib.allComponents.length + 1
const expectedComponentLinks = components.length + 1
expect(componentsList.length).toEqual(expectedComponentLinks)
})
})
Expand Down
4 changes: 2 additions & 2 deletions app/full-page-examples.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ module.exports = (app) => {
require('./views/full-page-examples/what-is-your-postcode')(app)
require('./views/full-page-examples/what-was-the-last-country-you-visited')(app)

app.get('/full-page-examples', (req, res, next) => {
res.locals.examples = fileHelper.fullPageExamples()
app.get('/full-page-examples', async (req, res, next) => {
res.locals.examples = await fileHelper.getFullPageExamples()

res.render('full-page-examples/index', (error, html) => {
if (error) {
Expand Down
8 changes: 4 additions & 4 deletions app/start.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const app = require('./app.js')
const configPaths = require('../config/paths.js')
const PORT = process.env.PORT || configPaths.ports.app

const app = require('./app.js')()
const PORT = process.env.PORT || configPaths.ports.app

app.listen(PORT, () => {
app().then(server => server.listen(PORT, () => {
console.log('Server started at http://localhost:' + PORT)
})
}))
5 changes: 5 additions & 0 deletions config/jest/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
env: {
jest: true
}
}
2 changes: 0 additions & 2 deletions config/jest/matchers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* eslint-env jest */

const { toHaveNoViolations } = require('jest-axe')

expect.extend(toHaveNoViolations)
3 changes: 2 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ module.exports = {
globalSetup: './config/jest/browser/open.mjs',
globalTeardown: './config/jest/browser/close.mjs'
}
]
],
testTimeout: 30000
}
131 changes: 103 additions & 28 deletions lib/file-helper.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,120 @@
const fs = require('fs')
const { readdir, readFile, stat } = require('fs/promises')
const path = require('path')
const yaml = require('js-yaml')
const fm = require('front-matter')

const configPaths = require('../config/paths.js')

const childDirectories = dir => {
return fs.readdirSync(dir)
.filter(file => fs.statSync(path.join(dir, file)).isDirectory())
/**
* Directory listing for path
*
* @param {string} directoryPath
* @returns {Promise<{ basename: string; stats: import('fs').Stats }[]>} entries
*/
const getListing = async (directoryPath) => {
const cache = getListing.cache ??= new Map()

// Cache listings
if (!cache.has(directoryPath)) {
const listing = await readdir(directoryPath)

// Loop through listing entries
const entries = listing.map(async basename => {
return { basename, stats: await stat(path.join(directoryPath, basename)) }
})

// Resolve on completion
cache.set(directoryPath, Promise.all(entries))
}

return cache.get(directoryPath)
}

// Generate component list from source directory, excluding anything that's not
// a directory (for example, .DS_Store files)
exports.allComponents = childDirectories(configPaths.components)
/**
* Directory listing (directories only)
*
* @param {string} directoryPath
* @returns {Promise<string[]>} directories
*/
const getDirectories = async (directoryPath) => {
const cache = getDirectories.cache ??= new Map()

// Cache directory-only listings
if (!cache.has(directoryPath)) {
const entries = await getListing(directoryPath)

// Read the contents of a file from a given path
const readFileContents = filePath => {
return fs.readFileSync(filePath, 'utf8')
cache.set(directoryPath, entries
.filter(({ stats }) => stats.isDirectory())
.map(({ basename: directory }) => directory))
}

return cache.get(directoryPath)
}

exports.readFileContents = readFileContents
/**
* Generate component list from source directory, excluding anything that's not
* a directory (for example, .DS_Store files)
*
* @returns {Promise<string[]>} directories
*/
const getAllComponents = async () => {
return getDirectories(configPaths.components)
}

const getComponentData = componentName => {
try {
const yamlPath = path.join(configPaths.components, componentName, `${componentName}.yaml`)
return yaml.load(
fs.readFileSync(yamlPath, 'utf8'), { json: true }
)
} catch (error) {
throw new Error(error)
/**
* Load component data
*
* @param {string} componentName - Component name
* @returns {Promise<{ examples?: unknown[]; params?: unknown[] }>} Component data
*/
const getComponentData = async componentName => {
const cache = getComponentData.cache ??= new Map()

// Cache component data
if (!cache.has(componentName)) {
let componentData = {}

try {
const yamlPath = path.join(configPaths.components, componentName, `${componentName}.yaml`)
componentData = yaml.load(await readFile(yamlPath, 'utf8'), { json: true })
} catch (error) {
throw new Error(error)
}

cache.set(componentName, componentData)
}

return cache.get(componentName)
}

exports.getComponentData = getComponentData
const getFullPageExamples = async () => {
if (!getFullPageExamples.cache) {
const examplesDirectories = await getDirectories(path.resolve(configPaths.fullPageExamples))

const examples = await Promise.all(
examplesDirectories.map(async folderName => {
const templatePath = path.join(configPaths.fullPageExamples, folderName, 'index.njk')
const { attributes } = fm(await readFile(templatePath, 'utf8'))

return {
name: folderName,
path: folderName,
...attributes
}
})
)

getFullPageExamples.cache = examples
.sort((a, b) => (a.name.toLowerCase() > b.name.toLowerCase()) ? 1 : -1)
}

return getFullPageExamples.cache
}

exports.fullPageExamples = () => {
return childDirectories(path.resolve(configPaths.fullPageExamples))
.map(folderName => ({
name: folderName,
path: folderName,
...fm(readFileContents(path.join(configPaths.fullPageExamples, folderName, 'index.njk'))).attributes
}))
.sort((a, b) => (a.name.toLowerCase() > b.name.toLowerCase()) ? 1 : -1)
module.exports = {
getAllComponents,
getFullPageExamples,
getComponentData,
getDirectories,
getListing
}
23 changes: 12 additions & 11 deletions lib/file-helper.unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,29 @@ const fileHelper = require('../lib/file-helper')

describe('getComponentData', () => {
it('returns an error if unable to load component data', () => {
expect(() => { fileHelper.getComponentData('not-a-real-component') }).toThrow(Error)
return expect(fileHelper.getComponentData('not-a-real-component'))
.rejects.toThrowError('Error: ENOENT: no such file or directory')
})

it('looks up the correct component path', () => {
it('looks up the correct component path', async () => {
jest.spyOn(path, 'join')

fileHelper.getComponentData('accordion')
await fileHelper.getComponentData('accordion')

expect(path.join).toHaveBeenCalledWith('src/govuk/components/', 'accordion', 'accordion.yaml')
})

it('outputs objects with an array of params and examples', () => {
const componentData = fileHelper.getComponentData('accordion')
it('outputs objects with an array of params and examples', async () => {
const componentData = await fileHelper.getComponentData('accordion')

expect(componentData).toEqual(expect.objectContaining({
params: expect.any(Array),
examples: expect.any(Array)
}))
})

it('outputs a param for each object with the expected attributes', () => {
const componentData = fileHelper.getComponentData('accordion')
it('outputs a param for each object with the expected attributes', async () => {
const componentData = await fileHelper.getComponentData('accordion')

componentData.params.forEach((param) => {
expect(param).toEqual(
Expand All @@ -38,8 +39,8 @@ describe('getComponentData', () => {
})
})

it('contains example objects with the expected attributes', () => {
const componentData = fileHelper.getComponentData('accordion')
it('contains example objects with the expected attributes', async () => {
const componentData = await fileHelper.getComponentData('accordion')

componentData.examples.forEach((example) => {
expect(example).toEqual(
Expand All @@ -53,8 +54,8 @@ describe('getComponentData', () => {
})

describe('fullPageExamples', () => {
it('contains name and path of each example, at a minimum', () => {
const fullPageExamples = fileHelper.fullPageExamples()
it('contains name and path of each example, at a minimum', async () => {
const fullPageExamples = await fileHelper.getFullPageExamples()

fullPageExamples.forEach((example) => {
expect(example).toEqual(
Expand Down
18 changes: 14 additions & 4 deletions lib/jest-helpers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const fs = require('fs')
const { readFile } = require('fs/promises')
const path = require('path')
const util = require('util')

Expand Down Expand Up @@ -135,8 +135,8 @@ async function renderAndInitialise (componentName, options = {}) {
* @param {string} componentName
* @returns {object} returns object that includes all examples at once
*/
function getExamples (componentName) {
const file = fs.readFileSync(
async function getExamples (componentName) {
const file = await readFile(
path.join(configPaths.components, componentName, `${componentName}.yaml`),
'utf8'
)
Expand Down Expand Up @@ -194,4 +194,14 @@ const axe = configureAxe({
}
})

module.exports = { axe, render, renderHtml, renderAndInitialise, getExamples, htmlWithClassName, renderSass, renderTemplate }
module.exports = {
axe,
getExamples,
htmlWithClassName,
nunjucksEnv,
render,
renderAndInitialise,
renderHtml,
renderSass,
renderTemplate
}
Loading

0 comments on commit efa705e

Please sign in to comment.