From 1c9cb136b6f2d2b68a13847e8b69816b7354a9ef Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 10:48:28 +0000 Subject: [PATCH 1/6] Test that components can be imported individually --- src/components/all.test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/components/all.test.js b/src/components/all.test.js index 09cad11299..f5d07da48f 100644 --- a/src/components/all.test.js +++ b/src/components/all.test.js @@ -1,5 +1,7 @@ /* eslint-env jest */ +const util = require('util') + const { allComponents } = require('../../lib/file-helper') const configPaths = require('../../config/paths.json') @@ -8,6 +10,13 @@ const configPaths = require('../../config/paths.json') // over the nunjucks environment. const nunjucks = require('nunjucks') +const sass = require('node-sass') +const sassRender = util.promisify(sass.render) + +const sassConfig = { + includePaths: [ configPaths.src ] +} + describe('When nunjucks is configured with a different base path', () => { beforeAll(() => { // Create a new Nunjucks environment that uses the src directory as its @@ -21,3 +30,10 @@ describe('When nunjucks is configured with a different base path', () => { }).not.toThrow() }) }) + +it.each(allComponents)('%s.scss renders to CSS without errors', (component) => { + return sassRender({ + file: `${configPaths.src}/components/${component}/_${component}.scss`, + ...sassConfig + }) +}) From 8b4c7c402cbebd61c42bf1e314339a895660354f Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 10:50:55 +0000 Subject: [PATCH 2/6] Test that core can be imported individually --- src/core/_global-styles.scss | 4 ++++ src/core/_links.scss | 4 ++++ src/core/_lists.scss | 4 ++++ src/core/_section-break.scss | 4 ++++ src/core/_template.scss | 2 ++ src/core/_typography.scss | 4 ++++ src/core/core.test.js | 15 +++++++++++++++ 7 files changed, 37 insertions(+) create mode 100644 src/core/core.test.js diff --git a/src/core/_global-styles.scss b/src/core/_global-styles.scss index 20d043a003..b5be687cef 100644 --- a/src/core/_global-styles.scss +++ b/src/core/_global-styles.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @import "links"; @import "typography"; diff --git a/src/core/_links.scss b/src/core/_links.scss index 9adedb5927..4f0b6584cb 100644 --- a/src/core/_links.scss +++ b/src/core/_links.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/core/links") { %govuk-link { diff --git a/src/core/_lists.scss b/src/core/_lists.scss index 40c58c071a..50ac6dc283 100644 --- a/src/core/_lists.scss +++ b/src/core/_lists.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/core/lists") { %govuk-list { diff --git a/src/core/_section-break.scss b/src/core/_section-break.scss index a316d31bf7..1626463b78 100644 --- a/src/core/_section-break.scss +++ b/src/core/_section-break.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/core/section-break") { %govuk-section-break { diff --git a/src/core/_template.scss b/src/core/_template.scss index 842b092b78..b9e6c50505 100644 --- a/src/core/_template.scss +++ b/src/core/_template.scss @@ -1,4 +1,6 @@ @import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; @include govuk-exports("govuk/core/template") { diff --git a/src/core/_typography.scss b/src/core/_typography.scss index db5bf99559..afee4a5a75 100644 --- a/src/core/_typography.scss +++ b/src/core/_typography.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/core/typography") { // Headings diff --git a/src/core/core.test.js b/src/core/core.test.js new file mode 100644 index 0000000000..2adf30ffdf --- /dev/null +++ b/src/core/core.test.js @@ -0,0 +1,15 @@ +/* eslint-env jest */ + +const util = require('util') +const glob = require('glob') + +const configPaths = require('../../config/paths.json') + +const sass = require('node-sass') +const sassRender = util.promisify(sass.render) + +const sassFiles = glob.sync(`${configPaths.src}/core/**/*.scss`) + +it.each(sassFiles)('%s renders to CSS without errors', (file) => { + return sassRender({ file: file, includePaths: [ configPaths.src ] }) +}) From c028b9a27e30f2fb673a731c595d54dbf296fd5d Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 10:51:00 +0000 Subject: [PATCH 3/6] Test that objects can be imported individually --- src/objects/_form-group.scss | 4 ++++ src/objects/_grid.scss | 4 ++++ src/objects/_main-wrapper.scss | 4 ++++ src/objects/_width-container.scss | 4 ++++ src/objects/objects.test.js | 15 +++++++++++++++ 5 files changed, 31 insertions(+) create mode 100644 src/objects/objects.test.js diff --git a/src/objects/_form-group.scss b/src/objects/_form-group.scss index cfe388d0e2..0a0b8e83cc 100644 --- a/src/objects/_form-group.scss +++ b/src/objects/_form-group.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/objects/form-group") { .govuk-form-group { diff --git a/src/objects/_grid.scss b/src/objects/_grid.scss index cdfece9b3d..34bcb4a030 100644 --- a/src/objects/_grid.scss +++ b/src/objects/_grid.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/objects/grid") { .govuk-grid-row { diff --git a/src/objects/_main-wrapper.scss b/src/objects/_main-wrapper.scss index bc7d74eb38..87cebac810 100644 --- a/src/objects/_main-wrapper.scss +++ b/src/objects/_main-wrapper.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + // Example usage with Breadcrumbs, phase banners, back links: //
// diff --git a/src/objects/_width-container.scss b/src/objects/_width-container.scss index cc1d562f77..996f1c1b37 100644 --- a/src/objects/_width-container.scss +++ b/src/objects/_width-container.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @mixin govuk-width-container { // Limit the width of the container to the page width max-width: $govuk-page-width; diff --git a/src/objects/objects.test.js b/src/objects/objects.test.js new file mode 100644 index 0000000000..ebbb48495b --- /dev/null +++ b/src/objects/objects.test.js @@ -0,0 +1,15 @@ +/* eslint-env jest */ + +const util = require('util') +const glob = require('glob') + +const configPaths = require('../../config/paths.json') + +const sass = require('node-sass') +const sassRender = util.promisify(sass.render) + +const sassFiles = glob.sync(`${configPaths.src}/objects/**/*.scss`) + +it.each(sassFiles)('%s renders to CSS without errors', (file) => { + return sassRender({ file: file, includePaths: [ configPaths.src ] }) +}) From bfe0e200e625c37264f31b233d4775fca211dfc1 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 10:57:48 +0000 Subject: [PATCH 4/6] Test that overrides can be imported individually --- src/overrides/_display.scss | 4 ++++ src/overrides/_spacing.scss | 4 ++++ src/overrides/_typography.scss | 4 ++++ src/overrides/_width.scss | 4 ++++ src/overrides/overrides.test.js | 15 +++++++++++++++ 5 files changed, 31 insertions(+) create mode 100644 src/overrides/overrides.test.js diff --git a/src/overrides/_display.scss b/src/overrides/_display.scss index 440fdc0d59..e818900635 100644 --- a/src/overrides/_display.scss +++ b/src/overrides/_display.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/overrides/display") { .govuk-\!-display-inline { diff --git a/src/overrides/_spacing.scss b/src/overrides/_spacing.scss index 7c21fa64d8..6ab7096be3 100644 --- a/src/overrides/_spacing.scss +++ b/src/overrides/_spacing.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + //// /// @group overrides //// diff --git a/src/overrides/_typography.scss b/src/overrides/_typography.scss index 56069ec1ab..9ccf2fcef6 100644 --- a/src/overrides/_typography.scss +++ b/src/overrides/_typography.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/overrides/typography") { // Font size and line height diff --git a/src/overrides/_width.scss b/src/overrides/_width.scss index 3fc9f8d2aa..1325a03765 100644 --- a/src/overrides/_width.scss +++ b/src/overrides/_width.scss @@ -1,3 +1,7 @@ +@import "../settings/all"; +@import "../tools/all"; +@import "../helpers/all"; + @include govuk-exports("govuk/overrides/width") { .govuk-\!-width-full { width: 100% !important; diff --git a/src/overrides/overrides.test.js b/src/overrides/overrides.test.js new file mode 100644 index 0000000000..69379e6ae7 --- /dev/null +++ b/src/overrides/overrides.test.js @@ -0,0 +1,15 @@ +/* eslint-env jest */ + +const util = require('util') +const glob = require('glob') + +const configPaths = require('../../config/paths.json') + +const sass = require('node-sass') +const sassRender = util.promisify(sass.render) + +const sassFiles = glob.sync(`${configPaths.src}/overrides/**/*.scss`) + +it.each(sassFiles)('%s renders to CSS without errors', (file) => { + return sassRender({ file: file, includePaths: [ configPaths.src ] }) +}) From 501e25dbd219524aafe78bac8e8e00bfd806c763 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 11:37:49 +0000 Subject: [PATCH 5/6] Simplify Sass render calls --- lib/jest-helpers.js | 24 +++++++++++++++++++++--- src/components/all.test.js | 15 +++------------ src/core/core.test.js | 8 ++------ src/objects/objects.test.js | 8 ++------ src/overrides/overrides.test.js | 8 ++------ 5 files changed, 30 insertions(+), 33 deletions(-) diff --git a/lib/jest-helpers.js b/lib/jest-helpers.js index 997ba94d39..a67d47e428 100644 --- a/lib/jest-helpers.js +++ b/lib/jest-helpers.js @@ -1,11 +1,16 @@ 'use strict' -const path = require('path') const fs = require('fs') -const nunjucks = require('nunjucks') +const path = require('path') +const util = require('util') + const cheerio = require('cheerio') +const nunjucks = require('nunjucks') const yaml = require('js-yaml') +const sass = require('node-sass') +const sassRender = util.promisify(sass.render) + const configPaths = require('../config/paths.json') const { componentNameToMacroName } = require('./helper-functions.js') @@ -65,6 +70,19 @@ function getExamples (componentName) { return examples } +/** + * Render Sass + * + * @param {object} options Options to pass to sass.render + * @return {promise} Result of calling sass.render + */ +function renderSass (options) { + return sassRender({ + includePaths: [ configPaths.src ], + ...options + }) +} + /** * Get the raw HTML representation of a component, and remove any other * child elements that do not match the component. @@ -83,4 +101,4 @@ function htmlWithClassName ($, className) { return $.html($component) } -module.exports = { render, getExamples, htmlWithClassName } +module.exports = { render, getExamples, htmlWithClassName, renderSass } diff --git a/src/components/all.test.js b/src/components/all.test.js index f5d07da48f..d4c64db15c 100644 --- a/src/components/all.test.js +++ b/src/components/all.test.js @@ -1,8 +1,7 @@ /* eslint-env jest */ -const util = require('util') - const { allComponents } = require('../../lib/file-helper') +const { renderSass } = require('../../lib/jest-helpers') const configPaths = require('../../config/paths.json') @@ -10,13 +9,6 @@ const configPaths = require('../../config/paths.json') // over the nunjucks environment. const nunjucks = require('nunjucks') -const sass = require('node-sass') -const sassRender = util.promisify(sass.render) - -const sassConfig = { - includePaths: [ configPaths.src ] -} - describe('When nunjucks is configured with a different base path', () => { beforeAll(() => { // Create a new Nunjucks environment that uses the src directory as its @@ -32,8 +24,7 @@ describe('When nunjucks is configured with a different base path', () => { }) it.each(allComponents)('%s.scss renders to CSS without errors', (component) => { - return sassRender({ - file: `${configPaths.src}/components/${component}/_${component}.scss`, - ...sassConfig + return renderSass({ + file: `${configPaths.src}/components/${component}/_${component}.scss` }) }) diff --git a/src/core/core.test.js b/src/core/core.test.js index 2adf30ffdf..d762435605 100644 --- a/src/core/core.test.js +++ b/src/core/core.test.js @@ -1,15 +1,11 @@ /* eslint-env jest */ -const util = require('util') const glob = require('glob') - +const { renderSass } = require('../../lib/jest-helpers') const configPaths = require('../../config/paths.json') -const sass = require('node-sass') -const sassRender = util.promisify(sass.render) - const sassFiles = glob.sync(`${configPaths.src}/core/**/*.scss`) it.each(sassFiles)('%s renders to CSS without errors', (file) => { - return sassRender({ file: file, includePaths: [ configPaths.src ] }) + return renderSass({ file: file }) }) diff --git a/src/objects/objects.test.js b/src/objects/objects.test.js index ebbb48495b..5bcc8246aa 100644 --- a/src/objects/objects.test.js +++ b/src/objects/objects.test.js @@ -1,15 +1,11 @@ /* eslint-env jest */ -const util = require('util') const glob = require('glob') - +const { renderSass } = require('../../lib/jest-helpers') const configPaths = require('../../config/paths.json') -const sass = require('node-sass') -const sassRender = util.promisify(sass.render) - const sassFiles = glob.sync(`${configPaths.src}/objects/**/*.scss`) it.each(sassFiles)('%s renders to CSS without errors', (file) => { - return sassRender({ file: file, includePaths: [ configPaths.src ] }) + return renderSass({ file: file }) }) diff --git a/src/overrides/overrides.test.js b/src/overrides/overrides.test.js index 69379e6ae7..59bed508ee 100644 --- a/src/overrides/overrides.test.js +++ b/src/overrides/overrides.test.js @@ -1,15 +1,11 @@ /* eslint-env jest */ -const util = require('util') const glob = require('glob') - +const { renderSass } = require('../../lib/jest-helpers') const configPaths = require('../../config/paths.json') -const sass = require('node-sass') -const sassRender = util.promisify(sass.render) - const sassFiles = glob.sync(`${configPaths.src}/overrides/**/*.scss`) it.each(sassFiles)('%s renders to CSS without errors', (file) => { - return sassRender({ file: file, includePaths: [ configPaths.src ] }) + return renderSass({ file: file }) }) From df54ea91c50b27e496bd6ac07dc2ed6130e8bcf8 Mon Sep 17 00:00:00 2001 From: Oliver Byford Date: Tue, 5 Mar 2019 14:29:09 +0000 Subject: [PATCH 6/6] Document in changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55747a9a35..b3f70190b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,23 @@ ([PR #N](https://github.com/alphagov/govuk-frontend/pull/N)) +- Ensure that files within the core, objects and overrides layers can be + imported individually + + Unlike components, the files within these layers did not previously import + their dependencies (for example, most of them require the govuk-exports mixin + but did not import it). + + We've also added tests to ensure that files within these layers can be + imported and rendered to CSS without erroring, which should catch this in the + future. + + Thanks to [Alasdair McLeay](https://github.com/penx) for originally raising a + PR to fix this. + + ([PR #1235](https://github.com/alphagov/govuk-frontend/pull/1235)) + + - Ensure inset component does not misalign nested components Thanks to [Paul Hayes](https://github.com/fofr) for raising this issue.