Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support html and njk extensions #2035

Merged
merged 2 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions __tests__/spec/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('error handling', () => {

afterEach(() => {
jest.restoreAllMocks()
require('../../lib/nunjucks/nunjucksLoader.js').stopWatchingNunjucks()
})

it('should show errors to the user in both the terminal and the browser', async () => {
Expand All @@ -42,6 +43,8 @@ describe('error handling', () => {

expect(response.status).toBe(500)
expect(response.text).toEqual('test error')

app.close()
})

it('shows an error if a template cannot be found', async () => {
Expand Down
3 changes: 3 additions & 0 deletions __tests__/spec/force-https-redirect.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ process.env.USE_HTTPS = 'true'
const app = require('../../server.js')

describe('The Prototype Kit - force HTTPS redirect functionality', () => {
afterAll(() => {
require('../../lib/nunjucks/nunjucksLoader.js').stopWatchingNunjucks()
})
describe('should in a production environment', () => {
it('have HTTP header "location" field that begins with https', async () => {
const response = await request(app).get('/')
Expand Down
2 changes: 1 addition & 1 deletion __tests__/spec/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ describe('migrate test prototype', () => {
'https://prototype-kit.service.gov.uk/docs/how-to-use-layouts\n' +
'#}\n' +
'\n' +
'{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}' + '\n'
'{% extends "govuk-prototype-kit/layouts/govuk-branded.njk" %}' + '\n'
)
})

Expand Down
4 changes: 4 additions & 0 deletions __tests__/spec/sanity-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ describe('The Prototype Kit', () => {
require('../../lib/build').generateAssetsSync()
}, createKitTimeout)

afterAll(() => {
app.close()
})

it('should call writeFileSync with result css from sass.compile', () => {
expect(fs.writeFileSync).toHaveBeenCalledWith(
path.join(projectDir, '.tmp', 'public', 'stylesheets', 'application.css'),
Expand Down
5 changes: 3 additions & 2 deletions __tests__/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ async function mkPrototype (prototypePath, {
kitPath,
overwrite = false,
allowTracking = undefined,
npmInstallLinks = undefined
npmInstallLinks = undefined,
commandLineParameters = ''
} = {}) { // TODO: Use kitPath if provided
HannahJMWood marked this conversation as resolved.
Show resolved Hide resolved
if (fs.existsSync(prototypePath)) {
if (!overwrite) {
Expand All @@ -78,7 +79,7 @@ async function mkPrototype (prototypePath, {
// Generate starter project
const repoDir = path.resolve(__dirname, '..', '..')
await exec(
`"${process.execPath}" bin/cli create --version local ${prototypePath}`,
`"${process.execPath}" bin/cli create --version local ${commandLineParameters} ${prototypePath}`,
{ cwd: repoDir, env: execEnv, stdio: 'inherit' }
)

Expand Down
52 changes: 48 additions & 4 deletions __tests__/utils/mock-file-system.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ function mockFileSystem (rootPath) {
return path.join(rootPath, partialPath)
}

const deleteFile = (pathParts) => {
const partialPath = path.join(...pathParts)
delete files[partialPath]
}

const deleteDirectory = (pathParts) => {
const partialPath = path.join(...pathParts)
delete directories[partialPath]
}

const doesDirectoryExist = (pathParts) => {
return directories.hasOwnProperty(path.join(...pathParts))
}
Expand All @@ -58,13 +68,14 @@ function mockFileSystem (rootPath) {
spiesToTearDown.pop().mockClear()
}
}
const getRootPath = () => rootPath
const setupSpies = () => {
const prepFilePath = filePath => {
return (filePath).replace(rootPath + path.sep, '').split(path.sec)
return (filePath).replace(rootPath + path.sep, '').split(path.sep)
}

const readFileImplementation = (filePath, encoding) => {
if (filePath.includes('node_modules/jest-worker')) {
if (filePath.includes('node_modules/jest-') || filePath.includes('node_modules/chalk')) {
return originalFsFunctions.promises.readFile.apply(null, arguments)
}
if (encoding !== 'utf8') {
Expand All @@ -78,13 +89,37 @@ function mockFileSystem (rootPath) {
}
}

const existsImplementation = filePath => doesFileExist(prepFilePath(filePath))
const existsImplementation = filePath => doesFileExist(prepFilePath(filePath)) || doesDirectoryExist(prepFilePath(filePath))

const writeFileImplementation = (filePath, content) => {
const trimmedPath = prepFilePath(filePath)
writeFile(trimmedPath, content)
}

const lstatImplementation = (filePath) => {
const fileExists = doesFileExist(prepFilePath(filePath))
const directoryExists = doesDirectoryExist(prepFilePath(filePath))
if (fileExists || directoryExists) {
return {
isDirectory: () => directoryExists
}
}
}

const readdirImplementation = (filePath) => {
const preppedPath = prepFilePath(filePath)
if (!doesDirectoryExist(preppedPath)) {
throw new Error('This is the wrong error, please make this error more accurate if you experience it. The directory doesn\'t exist')
}
const dirPathString = preppedPath.join(path.sep)
return [...Object.keys(directories), ...Object.keys(files)].filter(file => {
if (!file.startsWith(dirPathString)) {
return false
}
return dirPathString.split(path.sep).length === file.split(path.sep).length - 1
}).map(filename => filename.split(path.sep).at(-1))
}

const promiseWrap = fn => function () {
return new Promise((resolve, reject) => {
process.nextTick(() => {
Expand Down Expand Up @@ -118,8 +153,12 @@ function mockFileSystem (rootPath) {
jest.spyOn(fs, 'readFileSync').mockImplementation(readFileImplementation)
jest.spyOn(fs, 'writeFileSync').mockImplementation(writeFileImplementation)
jest.spyOn(fs, 'existsSync').mockImplementation(existsImplementation)
jest.spyOn(fs, 'lstatSync').mockImplementation(lstatImplementation)
jest.spyOn(fs, 'readdirSync').mockImplementation(readdirImplementation)
jest.spyOn(fs.promises, 'readFile').mockImplementation(promiseWrap(readFileImplementation))
jest.spyOn(fs.promises, 'writeFile').mockImplementation(promiseWrap(writeFileImplementation))
jest.spyOn(fs.promises, 'lstat').mockImplementation(promiseWrap(lstatImplementation))
jest.spyOn(fs.promises, 'readdir').mockImplementation(promiseWrap(readdirImplementation))
if (fs.promises.rm) {
jest.spyOn(fs.promises, 'rm').mockImplementation(promiseWrap(rm))
spiesToTearDown.push(fs.promises.rm)
Expand All @@ -133,17 +172,22 @@ function mockFileSystem (rootPath) {
spiesToTearDown.push(fs.readFileSync)
spiesToTearDown.push(fs.writeFileSync)
spiesToTearDown.push(fs.existsSync)
spiesToTearDown.push(fs.lstatSync)
spiesToTearDown.push(fs.promises.readFile)
spiesToTearDown.push(fs.promises.writeFile)
spiesToTearDown.push(fs.promises.lstat)
}
return {
writeFile,
deleteFile,
readFile,
doesFileExist,
createDirectory,
deleteDirectory,
doesDirectoryExist,
teardown,
setupSpies
setupSpies,
getRootPath
}
}

Expand Down
33 changes: 31 additions & 2 deletions bin/cli
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const { spawn, exec } = require('../lib/exec')
const { parse } = require('./utils/argv-parser')
const { prepareMigration, preflightChecks } = require('../migrator')
const { npmInstall, packageJsonFormat, getPackageVersionFromPackageJson, splitSemverVersion } = require('./utils')
const { recursiveDirectoryContentsSync } = require('../lib/utils')
const { appViewsDir } = require('../lib/utils/paths')

// Avoid requiring any kit server code at the top-level as we might want to
// change environment variables below.
Expand All @@ -17,7 +19,7 @@ const kitVersion = require('../package.json').version
const kitProjectName = require('../package.json').name

const argv = parse(process.argv, {
booleans: ['no-version-control', 'verbose', 'running-within-create-script']
booleans: ['no-version-control', 'verbose', 'running-within-create-script', 'use-njk-extensions']
})

const verboseLogger = !argv.options.verbose
Expand Down Expand Up @@ -258,6 +260,33 @@ async function runCreate () {
.then(displaySuccessMessage)
}

async function createStarterFiles (installDirectory) {
await fs.copy(path.join(kitRoot, 'prototype-starter'), installDirectory)

async function addToConfigFile (key, value) {
const configFileLocation = path.join(installDirectory, 'app', 'config.json')
const config = await fs.readJson(configFileLocation)
config[key] = value
await fs.writeJson(configFileLocation, config, { spaces: 2 })
}

function renameAllHtmlFilesToNjk () {
return recursiveDirectoryContentsSync(appViewsDir)
.filter(filePath => filePath.endsWith('.html'))
.map(filePath => fs.move(
path.join(appViewsDir, filePath),
path.join(appViewsDir, filePath.substring(0, filePath.length - '.html'.length) + '.njk')
))
}

if (argv.options['use-njk-extensions']) {
await Promise.all([
addToConfigFile('useNjkExtensions', true),
...renameAllHtmlFilesToNjk()
])
}
}

async function runInit () {
// `init` is stage two of the install process (see above), it should be
// called by `create` with the correct arguments.
Expand All @@ -273,7 +302,7 @@ async function runInit () {
const copyFile = (fileName) => fs.copy(path.join(kitRoot, fileName), path.join(installDirectory, fileName))

await Promise.all([
fs.copy(path.join(kitRoot, 'prototype-starter'), installDirectory),
createStarterFiles(installDirectory),
fs.writeFile(path.join(installDirectory, '.gitignore'), gitignore, 'utf8'),
fs.writeFile(path.join(installDirectory, '.npmrc'), npmrc, 'utf8'),
copyFile('LICENCE.txt'),
Expand Down
7 changes: 4 additions & 3 deletions cypress/e2e/dev/6-layout-tests/default-layout.cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const path = require('path')
const { waitForApplication } = require('../../utils')

const defaultLayoutFilePath = path.join('app', 'views', 'layouts', 'main.html')
const backupLayoutComment = '<!-- could not find layouts/main.html in prototype, using backup default template -->'
const backupLayoutComment = '<!-- could not find layouts/main.html or layouts/main.njk in prototype, using backup default template -->'

const comments = el => cy.wrap(
[...el.childNodes]
Expand All @@ -31,7 +31,8 @@ it('deleting default layout does not cause pages to fail to render', () => {
cy.visit('/', { failOnStatusCode: false })
cy.get('body').should('not.contains.text', 'Error: template not found')

cy.document().then(doc =>
cy.document().then(doc => {
cy.log('head content', doc.head.innerHTML)
comments(doc.head).should('contain', backupLayoutComment)
)
})
})
4 changes: 2 additions & 2 deletions govuk-prototype-kit.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"/lib/assets/javascripts/auto-store-data.js"
],
"importNunjucksMacrosInto": [
"/lib/nunjucks/govuk-prototype-kit/layouts/govuk-branded.html",
"/lib/nunjucks/govuk-prototype-kit/layouts/unbranded.html"
"/lib/nunjucks/govuk-prototype-kit/layouts/govuk-branded.njk",
"/lib/nunjucks/govuk-prototype-kit/layouts/unbranded.njk"
]
}
41 changes: 40 additions & 1 deletion lib/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const path = require('path')
// npm dependencies
const del = require('del')
const fse = require('fs-extra')
const fs = require('fs')
const sass = require('sass')

// local dependencies
Expand All @@ -19,8 +20,11 @@ const {
tmpDir,
tmpSassDir,
publicCssDir,
shadowNunjucksDir
shadowNunjucksDir,
backupNunjucksDir,
appViewsDir
} = require('./utils/paths')
const { recursiveDirectoryContentsSync } = require('./utils')
const { startPerformanceTimer, endPerformanceTimer } = require('./utils/performance')

const appSassOptions = {
Expand All @@ -41,14 +45,27 @@ function generateAssetsSync ({ verbose } = {}) {
clean()
sassPlugins()
autoImportComponentsSync()
createBackupNunjucksSync()
proxyUserSassIfItExists('application.scss')
proxyUserSassIfItExists('settings.scss')

generateCssSync()
endPerformanceTimer('generateAssetsSync', timer)
}

function cleanNunjucks () {
cleanShadowNunjucks()
cleanBackupNunjucks()
}
function cleanShadowNunjucks () {
del.sync(shadowNunjucksDir)
}
function cleanBackupNunjucks () {
del.sync(backupNunjucksDir)
}

function clean () {
cleanNunjucks()
// doesn't clean extensions.scss, should it?
// TODO: These files are deprecated, remove this line in v14?
del.sync(['public/**', '.port.tmp', '.tmp/port.tmp'])
Expand Down Expand Up @@ -149,6 +166,28 @@ function autoImportComponentsSync () {
endPerformanceTimer('autoImportComponentsSync', timer)
}

function createBackupNunjucksSync () {
const filesInViews = [
appViewsDir,
...plugins.getFileSystemPaths('nunjucksPaths')
].reverse().map(recursiveDirectoryContentsSync).flat()

function backupFilesWithExtension (fileExtension, backupExtension) {
const filesToBackup = filesInViews.filter(x => x.endsWith(fileExtension))
filesToBackup.forEach(fileName => {
const backupFilePath = path.join(backupNunjucksDir, fileName.substring(0, fileName.length - fileExtension.length) + backupExtension)
const includeMethod = fileName.split('/').includes('layouts') ? 'extends' : 'include'
const fileContents = `{% ${includeMethod} "${fileName}" %}`

fse.ensureDirSync(path.dirname(backupFilePath))
fs.writeFileSync(backupFilePath, fileContents, 'utf8')
})
}

backupFilesWithExtension('.njk', '.html')
backupFilesWithExtension('.html', '.njk')
}

module.exports = {
generateAssetsSync,
generateCssSync,
Expand Down
1 change: 1 addition & 0 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ function getConfig (config) {
overrideOrDefault('port', 'PORT', asNumber, 3000)
overrideOrDefault('useBrowserSync', 'USE_BROWSER_SYNC', asBoolean, true)
overrideOrDefault('useAutoStoreData', 'USE_AUTO_STORE_DATA', asBoolean, true)
overrideOrDefault('useNjkExtensions', 'USE_NJK_EXTENSIONS', asBoolean, false)
overrideOrDefault('logPerformance', 'LOG_PERFORMANCE', asBoolean, false)
overrideOrDefault('logPerformanceMatching', 'LOG_PERFORMANCE_MATCHING', asString, undefined)
overrideOrDefault('logPerformanceSummary', 'LOG_PERFORMANCE_SUMMARY', asNumber, undefined)
Expand Down
1 change: 1 addition & 0 deletions lib/config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ describe('config', () => {
isDevelopment: false,
isTest: true,
onGlitch: false,
useNjkExtensions: false,
logPerformance: false
})

Expand Down
5 changes: 4 additions & 1 deletion lib/dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ function runNodemon (port) {
// We also rely on this in acceptance tests
const onRestart = () => {
console.log('Restarting kit...')
console.log('')
}
// Warn about npm install on crash
const onCrash = () => {
Expand All @@ -96,9 +97,11 @@ function runNodemon (port) {
watch: [
path.join(projectDir, '.env'),
path.join(appDir, '**', '*.js'),
path.join(appDir, '**', '*.json')
path.join(appDir, '**', '*.json'),
path.join(projectDir, 'node_modules', 'govuk-prototype-kit', '**', '*.js')
],
script: path.join(packageDir, 'listen-on-port.js'),
ignoreRoot: ['.git'],
ignore: [
'app/assets/*'
]
Expand Down
8 changes: 8 additions & 0 deletions lib/final-backup-nunjucks/layouts/main.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{# this file exists as a backup in case the user has deleted their `app/views/layouts/main.html` template #}

{% extends "govuk-prototype-kit/layouts/govuk-branded.njk" %}

{% block meta %}
<!-- could not find layouts/main.html or layouts/main.njk in prototype, using backup default template -->
{{ super() }}
{% endblock %}
Loading