From c558637fdf2f36fda5212951f13aaf2b621c3b33 Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Tue, 20 Jun 2023 07:58:24 +0100 Subject: [PATCH 1/5] Migrate task arguments helper to CommonJS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These tests appear to use dynamic `import()` but really they’re transformed to `require()` by Jest to clear the “require cache” between test runs Since we’d like to start using _real_ dynamic `import()` in our Puppeteer tests (to import `govuk-frontend` as an ES module) the task arguments must be converted back to CommonJS See: https://jestjs.io/docs/ecmascript-modules --- packages/govuk-frontend/postcss.config.mjs | 2 +- .../helpers/{task-arguments.mjs => task-arguments.js} | 4 ++-- ...ments.unit.test.mjs => task-arguments.unit.test.js} | 10 +++++----- shared/tasks/npm.mjs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) rename shared/tasks/helpers/{task-arguments.mjs => task-arguments.js} (55%) rename shared/tasks/helpers/{task-arguments.unit.test.mjs => task-arguments.unit.test.js} (80%) diff --git a/packages/govuk-frontend/postcss.config.mjs b/packages/govuk-frontend/postcss.config.mjs index f725e8b4a8..d4e3ee2f8a 100644 --- a/packages/govuk-frontend/postcss.config.mjs +++ b/packages/govuk-frontend/postcss.config.mjs @@ -2,7 +2,7 @@ import autoprefixer from 'autoprefixer' import cssnano from 'cssnano' import cssnanoPresetDefault from 'cssnano-preset-default' import { pkg } from 'govuk-frontend-config' -import { isDev } from 'govuk-frontend-tasks/helpers/task-arguments.mjs' +import { isDev } from 'govuk-frontend-tasks/helpers/task-arguments.js' import postcss from 'postcss' import scss from 'postcss-scss' diff --git a/shared/tasks/helpers/task-arguments.mjs b/shared/tasks/helpers/task-arguments.js similarity index 55% rename from shared/tasks/helpers/task-arguments.mjs rename to shared/tasks/helpers/task-arguments.js index 420537dd8e..75333012ff 100644 --- a/shared/tasks/helpers/task-arguments.mjs +++ b/shared/tasks/helpers/task-arguments.js @@ -1,7 +1,7 @@ -import parser from 'yargs-parser' +const parser = require('yargs-parser') // Non-flag arguments as tasks const { _: tasks } = parser(process.argv) // Check for development task -export const isDev = tasks.includes('dev') +module.exports.isDev = tasks.includes('dev') diff --git a/shared/tasks/helpers/task-arguments.unit.test.mjs b/shared/tasks/helpers/task-arguments.unit.test.js similarity index 80% rename from shared/tasks/helpers/task-arguments.unit.test.mjs rename to shared/tasks/helpers/task-arguments.unit.test.js index 17e57e2b61..043e4a66a6 100644 --- a/shared/tasks/helpers/task-arguments.unit.test.mjs +++ b/shared/tasks/helpers/task-arguments.unit.test.js @@ -26,35 +26,35 @@ describe('Task arguments', () => { it('is flagged false', async () => { process.argv = [...argv] - const { isDev } = await import('./task-arguments.mjs') + const { isDev } = require('./task-arguments.js') expect(isDev).toBe(false) }) it("is flagged false for 'gulp build:app'", async () => { process.argv = [...argv, 'build:app'] - const { isDev } = await import('./task-arguments.mjs') + const { isDev } = require('./task-arguments.js') expect(isDev).toBe(false) }) it("is flagged false for 'gulp build:package'", async () => { process.argv = [...argv, 'build:package'] - const { isDev } = await import('./task-arguments.mjs') + const { isDev } = require('./task-arguments.js') expect(isDev).toBe(false) }) it("is flagged false for 'gulp build:release'", async () => { process.argv = [...argv, 'build:release'] - const { isDev } = await import('./task-arguments.mjs') + const { isDev } = require('./task-arguments.js') expect(isDev).toBe(false) }) it("is flagged true for 'gulp dev'", async () => { process.argv = [...argv, 'dev'] - const { isDev } = await import('./task-arguments.mjs') + const { isDev } = require('./task-arguments.js') expect(isDev).toBe(true) }) }) diff --git a/shared/tasks/npm.mjs b/shared/tasks/npm.mjs index 5bec8c9b94..90d1c39d63 100644 --- a/shared/tasks/npm.mjs +++ b/shared/tasks/npm.mjs @@ -2,7 +2,7 @@ import runScript from '@npmcli/run-script' import { paths } from 'govuk-frontend-config' import PluginError from 'plugin-error' -import { isDev } from './helpers/task-arguments.mjs' +import { isDev } from './helpers/task-arguments.js' import { task } from './index.mjs' /** From e5399f93859bfec55cae16586513eb1574aeeaef Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Tue, 20 Jun 2023 08:01:16 +0100 Subject: [PATCH 2/5] Prevent Babel converting dynamic `import()` to `require()` This fixes an issue running `import()` via Puppeteer by turning off dynamic import `require()` transforms that use Babel runtime helper --- babel.config.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/babel.config.js b/babel.config.js index eb14e52856..167bcdb65d 100644 --- a/babel.config.js +++ b/babel.config.js @@ -8,6 +8,9 @@ module.exports = function (api) { const presets = [ ['@babel/preset-env', { + exclude: [ + 'transform-dynamic-import' + ], targets: { node: 'current' } From 129d30b5fa89609282863ac823d71ff7101b19ae Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Thu, 22 Jun 2023 17:49:30 +0100 Subject: [PATCH 3/5] Update test boilerplate to use component preview layout --- packages/govuk-frontend-review/src/app.mjs | 2 +- .../src/views/component-preview.njk | 2 +- .../src/views/tests/boilerplate.njk | 25 ++++++++++--------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/govuk-frontend-review/src/app.mjs b/packages/govuk-frontend-review/src/app.mjs index c6e8ce4541..158014e4aa 100644 --- a/packages/govuk-frontend-review/src/app.mjs +++ b/packages/govuk-frontend-review/src/app.mjs @@ -112,7 +112,7 @@ export default async () => { const componentName = req.params.componentName const exampleName = req.params.exampleName || 'default' - const previewLayout = res.locals.componentData?.previewLayout || 'layout' + const previewLayout = res.locals.componentData?.previewLayout const exampleConfig = res.locals.componentData?.examples.find( example => example.name.replace(/ /g, '-') === exampleName diff --git a/packages/govuk-frontend-review/src/views/component-preview.njk b/packages/govuk-frontend-review/src/views/component-preview.njk index 9a7b52d1eb..96602d19b6 100644 --- a/packages/govuk-frontend-review/src/views/component-preview.njk +++ b/packages/govuk-frontend-review/src/views/component-preview.njk @@ -1,4 +1,4 @@ -{% extends "layouts/" + previewLayout + ".njk" %} +{% extends "layouts/" + previewLayout | default('layout') + ".njk" %} {% set bodyClasses %} {{ bodyClasses }} diff --git a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk index e8b9c179ed..e08fee3c4c 100644 --- a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk +++ b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk @@ -1,17 +1,18 @@ - - - - - Test Boilerplate - - - - +{% extends "component-preview.njk" %} + +{% set pageTitle = "Test boilerplate" %} +{% block pageTitle %}{{ pageTitle }} - GOV.UK{% endblock %} + +{% block content %} +

Test boilerplate

Used during testing to inject rendered components and test specific configurations

- - - +
+{% endblock %} + +{% block bodyEnd %} + +{% endblock %} From dadba0f227578824784cdf29500556f5a9924f7a Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Tue, 20 Jun 2023 08:09:16 +0100 Subject: [PATCH 4/5] Update test boilerplate to use component preview layout --- .../govuk-frontend-review/rollup.config.mjs | 2 +- .../src/javascripts/all.mjs | 5 +---- .../examples/scoped-initialisation/index.njk | 7 +++---- .../views/examples/template-custom/index.njk | 7 +++++-- .../views/examples/template-default/index.njk | 7 +++++-- .../src/views/examples/translated/index.njk | 17 +++++++++-------- .../src/views/layouts/_generic.njk | 7 +++++-- .../src/views/tests/boilerplate.njk | 2 +- .../govuk-frontend-review/tasks/scripts.mjs | 4 ++-- 9 files changed, 32 insertions(+), 26 deletions(-) diff --git a/packages/govuk-frontend-review/rollup.config.mjs b/packages/govuk-frontend-review/rollup.config.mjs index 5f11ad618b..4ba6b356e4 100644 --- a/packages/govuk-frontend-review/rollup.config.mjs +++ b/packages/govuk-frontend-review/rollup.config.mjs @@ -14,7 +14,7 @@ export default defineConfig(({ i: input }) => ({ * Output options */ output: { - format: 'iife', + format: 'es', /** * Output plugins diff --git a/packages/govuk-frontend-review/src/javascripts/all.mjs b/packages/govuk-frontend-review/src/javascripts/all.mjs index 77bd73ab2b..6a7fbfc08f 100644 --- a/packages/govuk-frontend-review/src/javascripts/all.mjs +++ b/packages/govuk-frontend-review/src/javascripts/all.mjs @@ -1,4 +1 @@ -import * as GOVUKFrontend from 'govuk-frontend' - -// @ts-expect-error Manually add globals to window for tests -window.GOVUKFrontend = GOVUKFrontend +export * from 'govuk-frontend' diff --git a/packages/govuk-frontend-review/src/views/examples/scoped-initialisation/index.njk b/packages/govuk-frontend-review/src/views/examples/scoped-initialisation/index.njk index 89aabf7a10..d96baf295b 100644 --- a/packages/govuk-frontend-review/src/views/examples/scoped-initialisation/index.njk +++ b/packages/govuk-frontend-review/src/views/examples/scoped-initialisation/index.njk @@ -45,11 +45,10 @@ {% endblock %} {% block bodyEnd %} - + {% endblock %} diff --git a/packages/govuk-frontend-review/src/views/examples/template-custom/index.njk b/packages/govuk-frontend-review/src/views/examples/template-custom/index.njk index a5e5e2a566..2838ef1ed2 100644 --- a/packages/govuk-frontend-review/src/views/examples/template-custom/index.njk +++ b/packages/govuk-frontend-review/src/views/examples/template-custom/index.njk @@ -121,7 +121,10 @@ {% block bodyEnd %} - - + + {% endblock %} diff --git a/packages/govuk-frontend-review/src/views/examples/template-default/index.njk b/packages/govuk-frontend-review/src/views/examples/template-default/index.njk index facd7b041d..7c72c80cff 100644 --- a/packages/govuk-frontend-review/src/views/examples/template-default/index.njk +++ b/packages/govuk-frontend-review/src/views/examples/template-default/index.njk @@ -12,7 +12,10 @@ {% block bodyEnd %} - - + + {% endblock %} diff --git a/packages/govuk-frontend-review/src/views/examples/translated/index.njk b/packages/govuk-frontend-review/src/views/examples/translated/index.njk index 15b87da586..f4c38ccb75 100644 --- a/packages/govuk-frontend-review/src/views/examples/translated/index.njk +++ b/packages/govuk-frontend-review/src/views/examples/translated/index.njk @@ -937,18 +937,19 @@ {% endblock %} {% block bodyEnd %} - + diff --git a/packages/govuk-frontend-review/src/views/layouts/_generic.njk b/packages/govuk-frontend-review/src/views/layouts/_generic.njk index 6a940053b3..f1d89284f3 100644 --- a/packages/govuk-frontend-review/src/views/layouts/_generic.njk +++ b/packages/govuk-frontend-review/src/views/layouts/_generic.njk @@ -15,6 +15,9 @@ {% set mainClasses = 'govuk-main-wrapper--auto-spacing' %} {% block bodyEnd %} - - + + {% endblock %} diff --git a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk index e08fee3c4c..f72ddce266 100644 --- a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk +++ b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk @@ -14,5 +14,5 @@ {% endblock %} {% block bodyEnd %} - + {% endblock %} diff --git a/packages/govuk-frontend-review/tasks/scripts.mjs b/packages/govuk-frontend-review/tasks/scripts.mjs index 9ec4badaf0..3ae26f46f4 100644 --- a/packages/govuk-frontend-review/tasks/scripts.mjs +++ b/packages/govuk-frontend-review/tasks/scripts.mjs @@ -18,9 +18,9 @@ export const compile = (options) => gulp.series( destPath: join(options.destPath, 'javascripts'), configPath: join(options.basePath, 'rollup.config.mjs'), - // Rename with `*.min.js` extension + // Rename with `*.bundle.min.mjs` extension filePath ({ dir, name }) { - return join(dir, `${name}.min.js`) + return join(dir, `${name}.bundle.min.mjs`) } }) ), From f1a0637111cb7cbda29dd6cf5a66228ac8504b29 Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Tue, 20 Jun 2023 08:11:00 +0100 Subject: [PATCH 5/5] Update Puppeteer tests to use ES modules not window globals An import map has been added to resolve `govuk-frontend` to `/javascripts/all.bundle.min.mjs` when run inside Puppeteer `page.evaluate()` --- package-lock.json | 1 + .../src/views/tests/boilerplate.njk | 7 ++++++ .../govuk/components/button/button.test.js | 8 +++---- .../character-count/character-count.test.js | 7 ++---- .../error-summary/error-summary.test.js | 12 +++++----- .../src/govuk/components/globals.test.mjs | 24 +++++++------------ shared/helpers/package.json | 1 + shared/helpers/puppeteer.js | 22 ++++++----------- shared/helpers/tsconfig.json | 9 ++++++- 9 files changed, 44 insertions(+), 47 deletions(-) diff --git a/package-lock.json b/package-lock.json index 37e505163d..d152bc3f86 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27789,6 +27789,7 @@ "devDependencies": { "@axe-core/puppeteer": "^4.7.3", "cheerio": "^1.0.0-rc.12", + "govuk-frontend": "*", "govuk-frontend-config": "*", "govuk-frontend-lib": "*", "jest-axe": "^7.0.1", diff --git a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk index f72ddce266..9832d919d2 100644 --- a/packages/govuk-frontend-review/src/views/tests/boilerplate.njk +++ b/packages/govuk-frontend-review/src/views/tests/boilerplate.njk @@ -3,6 +3,13 @@ {% set pageTitle = "Test boilerplate" %} {% block pageTitle %}{{ pageTitle }} - GOV.UK{% endblock %} +{% block head %} + {{ super() }} + +{% endblock %} + {% block content %}

Test boilerplate

diff --git a/packages/govuk-frontend/src/govuk/components/button/button.test.js b/packages/govuk-frontend/src/govuk/components/button/button.test.js index 0e302fc02c..96c6f9c602 100644 --- a/packages/govuk-frontend/src/govuk/components/button/button.test.js +++ b/packages/govuk-frontend/src/govuk/components/button/button.test.js @@ -18,14 +18,12 @@ describe('/components/button', () => { it('does not prevent further JavaScript from running', async () => { await goTo(page, '/tests/boilerplate') - const result = await page.evaluate((component) => { - const namespace = 'GOVUKFrontend' in window - ? window.GOVUKFrontend - : {} + const result = await page.evaluate(async (exportName) => { + const namespace = await import('govuk-frontend') // `undefined` simulates the element being missing, // from an unchecked `document.querySelector` for example - new namespace[component](undefined).init() + new namespace[exportName](undefined).init() // If our component initialisation breaks, this won't run return true diff --git a/packages/govuk-frontend/src/govuk/components/character-count/character-count.test.js b/packages/govuk-frontend/src/govuk/components/character-count/character-count.test.js index c3c2e6af78..0c0e8e603a 100644 --- a/packages/govuk-frontend/src/govuk/components/character-count/character-count.test.js +++ b/packages/govuk-frontend/src/govuk/components/character-count/character-count.test.js @@ -648,17 +648,14 @@ describe('Character count', () => { // Override maxlength to 10 maxlength: 10 }, - initialiser ({ config, namespace }) { - const $component = document.querySelector('[data-module]') - + initialiser ($module) { // Set locale to Welsh, which expects translations for 'one', 'two', // 'few' 'many' and 'other' forms – with the default English strings // provided we only have translations for 'one' and 'other'. // // We want to make sure we handle this gracefully in case users have // an existing character count inside an incorrect locale. - $component.setAttribute('lang', 'cy') - new namespace.CharacterCount($component, config).init() + $module.setAttribute('lang', 'cy') } }) diff --git a/packages/govuk-frontend/src/govuk/components/error-summary/error-summary.test.js b/packages/govuk-frontend/src/govuk/components/error-summary/error-summary.test.js index ba2211abbd..9321e1a1e1 100644 --- a/packages/govuk-frontend/src/govuk/components/error-summary/error-summary.test.js +++ b/packages/govuk-frontend/src/govuk/components/error-summary/error-summary.test.js @@ -1,4 +1,4 @@ -const { goToComponent, goToExample, renderAndInitialise } = require('govuk-frontend-helpers/puppeteer') +const { goToComponent, goToExample, renderAndInitialise, goTo } = require('govuk-frontend-helpers/puppeteer') const { getExamples } = require('govuk-frontend-lib/files') describe('Error Summary', () => { @@ -82,14 +82,14 @@ describe('Error Summary', () => { describe('using JavaScript configuration, with no elements on the page', () => { it('does not prevent further JavaScript from running', async () => { - const result = await page.evaluate((component) => { - const namespace = 'GOVUKFrontend' in window - ? window.GOVUKFrontend - : {} + await goTo(page, '/tests/boilerplate') + + const result = await page.evaluate(async (exportName) => { + const namespace = await import('govuk-frontend') // `undefined` simulates the element being missing, // from an unchecked `document.querySelector` for example - new namespace[component](undefined).init() + new namespace[exportName](undefined).init() // If our component initialisation breaks, this won't run return true diff --git a/packages/govuk-frontend/src/govuk/components/globals.test.mjs b/packages/govuk-frontend/src/govuk/components/globals.test.mjs index cc5b328865..22571a2514 100644 --- a/packages/govuk-frontend/src/govuk/components/globals.test.mjs +++ b/packages/govuk-frontend/src/govuk/components/globals.test.mjs @@ -5,23 +5,19 @@ describe('GOV.UK Frontend', () => { let exported beforeEach(async () => { - await goTo(page, '/') + await goTo(page, '/tests/boilerplate') - // Exported via global (e.g GOVUKFrontend.initAll) - exported = await page.evaluate(() => 'GOVUKFrontend' in window - ? Object.keys(window.GOVUKFrontend) - : undefined + // Exports available via browser dynamic import + exported = await page.evaluate(async () => + Object.keys(await import('govuk-frontend')) ) }) it('exports `initAll` function', async () => { - await goTo(page, '/') - - const typeofInitAll = await page.evaluate((utility) => { - const namespace = 'GOVUKFrontend' in window - ? window.GOVUKFrontend - : {} + await goTo(page, '/tests/boilerplate') + const typeofInitAll = await page.evaluate(async (utility) => { + const namespace = await import('govuk-frontend') return typeof namespace[utility] }, 'initAll') @@ -52,10 +48,8 @@ describe('GOV.UK Frontend', () => { const components = exported .filter(method => !['initAll', 'version'].includes(method)) - const componentsWithoutInitFunctions = await page.evaluate((components) => { - const namespace = 'GOVUKFrontend' in window - ? window.GOVUKFrontend - : {} + const componentsWithoutInitFunctions = await page.evaluate(async (components) => { + const namespace = await import('govuk-frontend') return components.filter(component => { const prototype = namespace[component].prototype diff --git a/shared/helpers/package.json b/shared/helpers/package.json index ddc4a983d8..3dcf9b14d8 100644 --- a/shared/helpers/package.json +++ b/shared/helpers/package.json @@ -16,6 +16,7 @@ "devDependencies": { "@axe-core/puppeteer": "^4.7.3", "cheerio": "^1.0.0-rc.12", + "govuk-frontend": "*", "govuk-frontend-config": "*", "govuk-frontend-lib": "*", "jest-axe": "^7.0.1", diff --git a/shared/helpers/puppeteer.js b/shared/helpers/puppeteer.js index c593bd6d75..18e549a390 100644 --- a/shared/helpers/puppeteer.js +++ b/shared/helpers/puppeteer.js @@ -76,9 +76,8 @@ async function axe (page, overrides = {}) { * @param {object} options - Render and initialise options * @param {object} options.params - Nunjucks macro params * @param {object} [options.config] - Component instantiation config - * @param {(context: { config?: object, namespace: object }) => void} [options.initialiser] - A function that'll run in the - * browser to execute arbitrary initialisation. Receives an object with the - * passed configuration as `config` and the GOVUKFrontend global as `namespace` + * @param {($module: Element) => void} [options.initialiser] - A function that'll run in the + * browser to execute arbitrary initialisation * @returns {Promise} Puppeteer page object */ async function renderAndInitialise (page, componentName, options) { @@ -92,22 +91,15 @@ async function renderAndInitialise (page, componentName, options) { }, html) // Run a script to init the JavaScript component - await page.evaluate((componentClassName, options) => { - const $component = document.querySelector('[data-module]') - - // Check for window global - if (!('GOVUKFrontend' in window) || !window.GOVUKFrontend[componentClassName]) { - throw new Error(`Global 'window.GOVUKFrontend.${componentClassName}' not found`) - } + await page.evaluate(async (exportName, options) => { + const $module = document.querySelector('[data-module]') if (options.initialiser) { - return options.initialiser({ - config: options.config, - namespace: window.GOVUKFrontend - }) + options.initialiser($module) } - new window.GOVUKFrontend[componentClassName]($component, options.config).init() + const namespace = await import('govuk-frontend') + new namespace[exportName]($module, options.config).init() }, componentNameToClassName(componentName), options) return page diff --git a/shared/helpers/tsconfig.json b/shared/helpers/tsconfig.json index 4073f258b8..a711d76daf 100644 --- a/shared/helpers/tsconfig.json +++ b/shared/helpers/tsconfig.json @@ -1,4 +1,11 @@ { "extends": "../../tsconfig.base.json", - "include": ["**/*.js", "**/*.mjs", "../config", "../lib"] + "include": [ + "**/*.js", + "**/*.mjs", + "../config", + "../lib", + "../../packages/govuk-frontend" + ], + "exclude": ["./dist/**", "../../packages/govuk-frontend/dist/**"] }