From 30f581b47ccd8c2510d9e98cc71ae7116b49c676 Mon Sep 17 00:00:00 2001 From: Vanita Barrett Date: Tue, 9 Nov 2021 13:39:52 +0000 Subject: [PATCH 1/5] Switch JS compile to use forEach loop Changes the existing JS compile gulp task to use `glob` and iterate over each file using `forEach`. --- tasks/gulp/compile-assets.js | 59 +++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/tasks/gulp/compile-assets.js b/tasks/gulp/compile-assets.js index b69b59dd5c..834d416f3b 100644 --- a/tasks/gulp/compile-assets.js +++ b/tasks/gulp/compile-assets.js @@ -13,6 +13,7 @@ const taskArguments = require('./task-arguments') const gulpif = require('gulp-if') const uglify = require('gulp-uglify') const eol = require('gulp-eol') +const glob = require('glob') const rename = require('gulp-rename') const cssnano = require('cssnano') const postcsspseudoclasses = require('postcss-pseudo-classes')({ @@ -185,31 +186,35 @@ gulp.task('scss:compile', function (done) { // Compile js task for preview ---------- // -------------------------------------- -gulp.task('js:compile', () => { - // for dist/ folder we only want compiled 'all.js' file - const srcFiles = isDist ? configPaths.src + 'all.js' : configPaths.src + '**/*.js' - - return gulp.src([ - srcFiles, - '!' + configPaths.src + '**/*.test.js' - ]) - .pipe(rollup({ - // Used to set the `window` global and UMD/AMD export name. - name: 'GOVUKFrontend', - // Legacy mode is required for IE8 support - legacy: true, - // UMD allows the published bundle to work in CommonJS and in the browser. - format: 'umd' - })) - .pipe(gulpif(isDist, uglify({ - ie8: true - }))) - .pipe(gulpif(isDist, - rename({ - basename: 'govuk-frontend', - extname: '.min.js' - }) - )) - .pipe(eol()) - .pipe(gulp.dest(destinationPath)) +gulp.task('js:compile', (done) => { + // for dist/ folder we only want compiled 'all.js' + const fileLookup = isDist ? configPaths.src + 'all.js' : configPaths.src + '**/!(*.test).js' + const srcFiles = glob.sync(fileLookup) + + srcFiles.forEach(function (file) { + const currentDirectory = path.dirname(file) + const newDirectory = currentDirectory.replace('src/govuk', '') + + return gulp.src(file) + .pipe(rollup({ + // Used to set the `window` global and UMD/AMD export name. + name: 'GOVUKFrontend', + // Legacy mode is required for IE8 support + legacy: true, + // UMD allows the published bundle to work in CommonJS and in the browser. + format: 'umd' + })) + .pipe(gulpif(isDist, uglify({ + ie8: true + }))) + .pipe(gulpif(isDist, + rename({ + basename: 'govuk-frontend', + extname: '.min.js' + }) + )) + .pipe(eol()) + .pipe(gulp.dest(destinationPath() + newDirectory)) + }) + done() }) From f8b03f236a8d4e98f2f9bba0c43c9c8ee72d08e4 Mon Sep 17 00:00:00 2001 From: Vanita Barrett Date: Mon, 15 Nov 2021 17:03:46 +0000 Subject: [PATCH 2/5] Add helper function for generating module name Add a new function to our helper-functions.js that converts a component name to the Javascript module name. This means we can use the function elsewhere, such as tests, without duplicating the logic. It also makes sense to put together with the function that generates the macro names, as the two are linked --- lib/helper-functions.js | 20 ++++++++++++++++++++ tasks/gulp/compile-assets.js | 2 ++ 2 files changed, 22 insertions(+) diff --git a/lib/helper-functions.js b/lib/helper-functions.js index 7cb65cce2c..f70040a8e4 100644 --- a/lib/helper-functions.js +++ b/lib/helper-functions.js @@ -19,3 +19,23 @@ const componentNameToMacroName = componentName => { return `govuk${macroName}` } exports.componentNameToMacroName = componentNameToMacroName + +// Convert component name to JavaScript UMD module name +// +// This helper function takes a component name and returns the corresponding +// module name, which is used by rollup to set the `window` global and UMD/AMD export name +// +// Component names are lowercase, dash-separated strings (button, date-input), +// whilst module names have a `GOVUKFrontend.` prefix and are pascal cased (GOVUKFrontend.Button, +// GOVUKFrontend.CharacterCount). +const componentNameToJavaScriptModuleName = componentName => { + const macroName = componentName + .toLowerCase() + .split('-') + // capitalize each 'word' + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) + .join('') + + return `GOVUKFrontend.${macroName}` +} +exports.componentNameToJavaScriptModuleName = componentNameToJavaScriptModuleName diff --git a/tasks/gulp/compile-assets.js b/tasks/gulp/compile-assets.js index 834d416f3b..2542712336 100644 --- a/tasks/gulp/compile-assets.js +++ b/tasks/gulp/compile-assets.js @@ -1,5 +1,7 @@ 'use strict' +const helperFunctions = require('../../lib/helper-functions') + const path = require('path') const gulp = require('gulp') From 4abb8ab447e8e2893aacbb1eef5bcf794f5e14d5 Mon Sep 17 00:00:00 2001 From: Vanita Barrett Date: Mon, 8 Nov 2021 16:41:16 +0000 Subject: [PATCH 3/5] Add module name to the UMD export name Adds the module name (e.g: Accordion) to the UMD export name. For example: GOVUKFrontend becomes GOVUKFrontend.Accordion If the file is all.js (which pulls in all component JS), fall back to the original naming ("GOVUKFrontend") otherwise the modules are nested underneath an `all`, e.g: GOVUKFrontend.all.initAll() --- tasks/gulp/compile-assets.js | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tasks/gulp/compile-assets.js b/tasks/gulp/compile-assets.js index 2542712336..54a0784a42 100644 --- a/tasks/gulp/compile-assets.js +++ b/tasks/gulp/compile-assets.js @@ -1,6 +1,6 @@ 'use strict' -const helperFunctions = require('../../lib/helper-functions') +const { componentNameToJavaScriptModuleName } = require('../../lib/helper-functions') const path = require('path') @@ -189,18 +189,28 @@ gulp.task('scss:compile', function (done) { // Compile js task for preview ---------- // -------------------------------------- gulp.task('js:compile', (done) => { - // for dist/ folder we only want compiled 'all.js' + // For dist/ folder we only want compiled 'all.js' const fileLookup = isDist ? configPaths.src + 'all.js' : configPaths.src + '**/!(*.test).js' + + // Perform a synchronous search and return an array of matching file names const srcFiles = glob.sync(fileLookup) srcFiles.forEach(function (file) { - const currentDirectory = path.dirname(file) - const newDirectory = currentDirectory.replace('src/govuk', '') + // This is combined with desinationPath in gulp.dest() + // so the files are output to the correct folders + const newDirectoryPath = path.dirname(file).replace('src/govuk', '') + + // We only want to give component JavaScript a unique module name + let moduleName = 'GOVUKFrontend' + if (path.dirname(file).includes('/components/')) { + moduleName = componentNameToJavaScriptModuleName(path.parse(file).name) + } return gulp.src(file) .pipe(rollup({ - // Used to set the `window` global and UMD/AMD export name. - name: 'GOVUKFrontend', + // Used to set the `window` global and UMD/AMD export name + // Component JavaScript is given a unique name to aid individual imports, e.g GOVUKFrontend.Accordion + name: moduleName, // Legacy mode is required for IE8 support legacy: true, // UMD allows the published bundle to work in CommonJS and in the browser. @@ -216,7 +226,7 @@ gulp.task('js:compile', (done) => { }) )) .pipe(eol()) - .pipe(gulp.dest(destinationPath() + newDirectory)) + .pipe(gulp.dest(destinationPath() + newDirectoryPath)) }) done() }) From 952c4f79f1a8d75c97fcc9e591b29fe91de73356 Mon Sep 17 00:00:00 2001 From: Vanita Barrett Date: Fri, 12 Nov 2021 13:46:42 +0000 Subject: [PATCH 4/5] Add to Changelog --- CHANGELOG.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6aed344cd8..98be0aee07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,29 @@ If you see unexpected behaviour, make sure the `id` for the textarea is unique This change was introduced in [pull request #2408: Prevent issues with character count when textarea `id` includes CSS syntax characters](https://github.com/alphagov/govuk-frontend/pull/2408). +#### Make sure individually imported JavaScript modules work as expected + +You do not need to do anything if you have either: + +- followed our [Getting Started guide](https://frontend.design-system.service.gov.uk/get-started/#5-get-the-javascript-working) and are importing all of the GOV.UK Frontend JavaScript in one go via `all.js` +- installed GOV.UK Frontend using precompiled files + +We've changed the naming of our components' JavaScript modules so that individual imports are now attached to +`window.GOVUKFrontend.[ComponentName]` instead of `window.GOVUKFrontend`. + +You can now import multiple modules without overwriting the previous one, for example: + +``` +//= require govuk/components/accordion/accordion.js +//= require govuk/components/button/button.js + +# These modules are available under window.GOVUKFrontend.Accordion and window.GOVUKFrontend.Button respectively +``` + +If you're importing JavaScript modules individually, you should check any references to `window.GOVUKFrontend` in your code and update them to point to the correct component, `window.GOVUKFrontend.[ComponentName]`. You can now remove any workarounds you may have implemented. + +This change was introduced in [pull request #1836: Rename exported JavaScript modules to include component name](https://github.com/alphagov/govuk-frontend/issues/1836)]. + #### Remove calls to deprecated `iff` Sass function We've removed the `iff` function which we deprecated in [GOV.UK Frontend version 3.6.0](https://github.com/alphagov/govuk-frontend/releases/tag/v3.6.0). @@ -149,6 +172,7 @@ We’ve made fixes to GOV.UK Frontend in the following pull requests: - [#2370: Prevent issues with conditionally revealed content when content `id` includes CSS syntax characters](https://github.com/alphagov/govuk-frontend/pull/2370) - [#2408: Prevent issues with character count when textarea `id` includes CSS syntax characters](https://github.com/alphagov/govuk-frontend/pull/2408) - [#2434: Add brand colour for Department for Levelling Up, Housing and Communities (DLUHC)](https://github.com/alphagov/govuk-frontend/pull/2434) +- [#1836: Rename exported JavaScript modules to include component name](https://github.com/alphagov/govuk-frontend/issues/1836)) ## 3.14.0 (Feature release) From ad45adf346cc34522928a7167d80df3a6b18ea7a Mon Sep 17 00:00:00 2001 From: Vanita Barrett Date: Mon, 15 Nov 2021 17:08:25 +0000 Subject: [PATCH 5/5] Add tests for new module names --- .../__tests__/after-build-package.test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tasks/gulp/__tests__/after-build-package.test.js b/tasks/gulp/__tests__/after-build-package.test.js index 99284c71da..a1b06866ab 100644 --- a/tasks/gulp/__tests__/after-build-package.test.js +++ b/tasks/gulp/__tests__/after-build-package.test.js @@ -9,11 +9,13 @@ var glob = require('glob') const configPaths = require('../../../config/paths.json') const lib = require('../../../lib/file-helper') +const { componentNameToJavaScriptModuleName } = require('../../../lib/helper-functions') const { renderSass } = require('../../../lib/jest-helpers') const readFile = util.promisify(fs.readFile) const componentNames = lib.allComponents.slice() +const componentsWithJavaScript = glob.sync(configPaths.package + 'govuk/components/' + '**/!(*.test).js') describe('package/', () => { it('should contain the expected files', () => { @@ -101,6 +103,19 @@ describe('package/', () => { }) }) + describe('all.js', () => { + it('should have correct module name', async () => { + const allJsFile = path.join(configPaths.package, 'govuk', 'all.js') + return readFile(allJsFile, 'utf8') + .then((data) => { + expect(data).toContain("typeof define === 'function' && define.amd ? define('GOVUKFrontend', ['exports'], factory)") + }) + .catch(error => { + throw error + }) + }) + }) + describe('component', () => { it.each(componentNames)('\'%s\' should have macro-options.json that contains JSON', (name) => { const filePath = path.join(configPaths.package, 'govuk', 'components', name, 'macro-options.json') @@ -127,6 +142,20 @@ describe('package/', () => { }) }) + describe('components with JavaScript', () => { + it.each(componentsWithJavaScript)('\'%s\' should have component JavaScript file with correct module name', (javaScriptFile) => { + const moduleName = componentNameToJavaScriptModuleName(path.parse(javaScriptFile).name) + + return readFile(javaScriptFile, 'utf8') + .then((data) => { + expect(data).toContain("typeof define === 'function' && define.amd ? define('" + moduleName + "', factory)") + }) + .catch(error => { + throw error + }) + }) + }) + describe('fixtures', () => { it.each(componentNames)('\'%s\' should have fixtures.json that contains JSON', (name) => { const filePath = path.join(configPaths.package, 'govuk', 'components', name, 'fixtures.json')