diff --git a/CHANGELOG.md b/CHANGELOG.md index cf678263b2..c144b49248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +### New features + +- [#2049: Migrate changed files if possible](https://github.com/alphagov/govuk-prototype-kit/pull/2049) + Converts the following files if possible during migration: + - application.js + - filters.js + ## 13.5.0 ### New features diff --git a/__tests__/fixtures/test-v11-prototype/app/assets/javascripts/application.js b/__tests__/fixtures/test-v11-prototype/app/assets/javascripts/application.js index 241bec763f..e36ceb58d9 100644 --- a/__tests__/fixtures/test-v11-prototype/app/assets/javascripts/application.js +++ b/__tests__/fixtures/test-v11-prototype/app/assets/javascripts/application.js @@ -1,4 +1,5 @@ /* global $ */ +/* global GOVUK */ // Warn about using the kit in production if (window.console && window.console.info) { @@ -7,4 +8,16 @@ if (window.console && window.console.info) { $(document).ready(function () { window.GOVUKFrontend.initAll() + // Use GOV.UK shim-links-with-button-role.js to trigger a link styled to look like a button, + // with role="button" when the space key is pressed. + GOVUK.shimLinksWithButtonRole.init() + + // Details/summary polyfill from frontend toolkit + GOVUK.details.init() + + // Show and hide toggled content + // Where .multiple-choice uses the data-target attribute + // to toggle hidden content + var showHideContent = new GOVUK.ShowHideContent() + showHideContent.init() }) diff --git a/__tests__/spec/migrate.js b/__tests__/spec/migrate.js index d99b84c166..60b324e3cf 100644 --- a/__tests__/spec/migrate.js +++ b/__tests__/spec/migrate.js @@ -112,16 +112,29 @@ describe('migrate test prototype', () => { it('application.js should be overwritten', () => { const jsFileContents = getNormalisedFileContent(path.join(assetsDirectory, 'javascripts', 'application.js')) - expect(jsFileContents).toEqual( - '//\n' + - '// For guidance on how to add JavaScript see:\n' + - '// https://prototype-kit.service.gov.uk/docs/adding-css-javascript-and-images\n' + - '//\n' + - '\n' + - 'window.GOVUKPrototypeKit.documentReady(() => {' + '\n' + - ' // Add JavaScript here' + '\n' + - '})' + '\n' - ) + expect(jsFileContents).toEqual(`/* global GOVUK */ + +// +// For guidance on how to add JavaScript see: +// https://prototype-kit.service.gov.uk/docs/adding-css-javascript-and-images +// + + +window.GOVUKPrototypeKit.documentReady(function () { + // Use GOV.UK shim-links-with-button-role.js to trigger a link styled to look like a button, + // with role="button" when the space key is pressed. + GOVUK.shimLinksWithButtonRole.init() + + // Details/summary polyfill from frontend toolkit + GOVUK.details.init() + + // Show and hide toggled content + // Where .multiple-choice uses the data-target attribute + // to toggle hidden content + var showHideContent = new GOVUK.ShowHideContent() + showHideContent.init() +}) +`) }) it('application.scss should be updated correctly', () => { diff --git a/migrator/file-helpers.js b/migrator/file-helpers.js index 3a191167ce..ad0f8f8dfa 100644 --- a/migrator/file-helpers.js +++ b/migrator/file-helpers.js @@ -20,7 +20,7 @@ const argv = parse(process.argv, { async function verboseLog () { await log(...arguments) - if (process.env.GPK_UPGRADE_DEBUG !== 'true' || argv.options.verbose) { + if (process.env.GPK_UPGRADE_DEBUG !== 'true' || !argv.options.verbose) { return } console.log('[debug]', ...arguments) diff --git a/migrator/index.js b/migrator/index.js index 04ffb58858..f7fd895a6d 100644 --- a/migrator/index.js +++ b/migrator/index.js @@ -18,7 +18,8 @@ const { upgradeIfUnchanged, deleteIfUnchanged, removeOldPatternIncludesFromSassFile, - updateUnbrandedLayouts + updateUnbrandedLayouts, + upgradeIfPossible } = require('./migration-steps') const supportPage = 'https://prototype-kit.service.gov.uk/docs/support' @@ -142,7 +143,7 @@ async function migrate () { prepareSass('app/assets/sass/application.scss'), deleteUnusedFiles(filesToDelete), deleteUnusedDirectories(directoriesToDelete), - upgradeIfUnchanged(filesToUpdateIfUnchanged), + upgradeIfUnchanged(filesToUpdateIfUnchanged, '', upgradeIfPossible), upgradeLayoutIfUnchanged(), updateUnbrandedLayouts('app/views'), deleteIfUnchanged(filesToDeleteIfUnchanged), diff --git a/migrator/migration-steps.js b/migrator/migration-steps.js index 8d2f5fbba1..f1786ebfb2 100644 --- a/migrator/migration-steps.js +++ b/migrator/migration-steps.js @@ -25,6 +25,7 @@ const { deleteDirectoryIfEmpty, writeFileLinesToFile } = require('./file-helpers') +const { upgradeIfPossible } = require('./upgrade-steps') // Allows mocking of getOldConfig const getOldConfig = (oldConfigPath) => config.getConfig(require(path.join(projectDir, oldConfigPath))) @@ -215,11 +216,9 @@ async function upgradeIfUnchanged (filePaths, starterFilePath, additionalStep) { try { if (matchFound) { await copyFileFromStarter(starterFilePath || filePath, filePath) - } else { - await reporter(false) } if (additionalStep) { - result = await additionalStep() + result = await additionalStep(filePath, matchFound) } else { result = matchFound } @@ -254,5 +253,6 @@ module.exports = { deleteEmptyDirectories, deleteIfUnchanged, upgradeIfUnchanged, - updateUnbrandedLayouts + updateUnbrandedLayouts, + upgradeIfPossible } diff --git a/migrator/migration-steps.spec.js b/migrator/migration-steps.spec.js index 3cd4057931..d84ad9a0c1 100644 --- a/migrator/migration-steps.spec.js +++ b/migrator/migration-steps.spec.js @@ -50,7 +50,14 @@ const { projectDir, starterDir, appDir } = require('../lib/utils/paths') const migrationSteps = require('./migration-steps') const { preflightChecks, deleteIfUnchanged, removeOldPatternIncludesFromSassFile } = require('./migration-steps') -const { migrateConfig, prepareAppRoutes, prepareSass, deleteUnusedFiles, deleteUnusedDirectories, upgradeIfUnchanged } = migrationSteps +const { + migrateConfig, + prepareAppRoutes, + prepareSass, + deleteUnusedFiles, + deleteUnusedDirectories, + upgradeIfUnchanged +} = migrationSteps describe('migration steps', () => { const mockReporter = reporter.mockReporter @@ -310,7 +317,7 @@ describe('migration steps', () => { expect(reporter.addReporter).toHaveBeenCalledTimes(1) expect(reporter.addReporter).toHaveBeenCalledWith(`Overwrite ${layout}`) - expect(mockReporter).toHaveBeenCalledTimes(2) + expect(mockReporter).toHaveBeenCalledTimes(1) expect(mockReporter).toHaveBeenCalledWith(true) }) diff --git a/migrator/migrator.spec.js b/migrator/migrator.spec.js index 00f157d68c..fb3820d66b 100644 --- a/migrator/migrator.spec.js +++ b/migrator/migrator.spec.js @@ -21,6 +21,7 @@ jest.mock('./migration-steps', () => { deleteUnusedDirectories: jest.fn().mockResolvedValue(true), deleteEmptyDirectories: jest.fn().mockResolvedValue([true]), upgradeIfUnchanged: jest.fn(), + upgradeIfPossible: jest.fn(), updateUnbrandedLayouts: jest.fn().mockResolvedValue(true), deleteIfUnchanged: jest.fn().mockResolvedValue([true, true, true]), removeOldPatternIncludesFromSassFile: jest.fn().mockResolvedValue(true) diff --git a/migrator/upgrade-steps.js b/migrator/upgrade-steps.js new file mode 100644 index 0000000000..543ac74ba0 --- /dev/null +++ b/migrator/upgrade-steps.js @@ -0,0 +1,190 @@ +// core dependencies +const path = require('path') +const fsp = require('fs').promises + +// local dependencies +const { starterDir, projectDir } = require('../lib/utils/paths') +const { addReporter } = require('./reporter') + +function removeMatchedText (originalContent, matchText) { + const originalContentLines = originalContent.split('\n') + const newContentLines = [] + + // Remove the matchText from the original code + while (originalContentLines.length) { + const match = matchText.findIndex((text) => text.every((line, index) => { + const originalContentLine = originalContentLines[index] + return line === originalContentLine?.trim() + })) + if (match === -1) { + newContentLines.push(originalContentLines.shift()) + } else { + matchText[match].forEach(() => originalContentLines.shift()) + } + } + return newContentLines.join('\n') +} + +function occurencesOf (searchText, text) { + return text.split(searchText).length - 1 +} + +async function upgradeApplicationJs (fullPath, reporter) { + const commentText = `// +// For guidance on how to add JavaScript see: +// https://prototype-kit.service.gov.uk/docs/adding-css-javascript-and-images +//` + const matchText = [ + [ + '// Warn about using the kit in production', + 'if (window.console && window.console.info) {', + 'window.console.info(\'GOV.UK Prototype Kit - do not use for production\')', + '}' + ], + ['window.GOVUKFrontend.initAll()'] + ] + const jqueryReadyText = '$(document).ready(' + const kitReadyText = 'window.GOVUKPrototypeKit.documentReady(' + const originalContent = await fsp.readFile(fullPath, 'utf8') + + // Remove the matchText from the original code + let modifiedContent = removeMatchedText(originalContent, matchText) + if (occurencesOf(jqueryReadyText, modifiedContent) === occurencesOf('$.', modifiedContent) + occurencesOf('$(', modifiedContent)) { + modifiedContent = modifiedContent.replaceAll(jqueryReadyText, kitReadyText) + // Remove if jQuery is not necessary + if (!modifiedContent.includes('$(') && !modifiedContent.includes('$.')) { + modifiedContent = modifiedContent.replaceAll(/\/\*\s*global\s+\$\s?\*\/[\s\r\n]*/g, '') + } + } + + const newContentLines = [] + let commentInserted = false + + const modifiedContentLines = modifiedContent.split('\n') + + modifiedContentLines.forEach((line, index) => { + if (!commentInserted) { + if (![ + /\/\*\s*global\s+/, + /"use strict"/ + ].some((regex) => line.search(regex) > -1)) { + if (index > 0) { + newContentLines.push('') + } + commentText.split('\n').forEach((commentLine) => newContentLines.push(commentLine)) + // insert a blank line after the comments if it's not the first line + commentInserted = true + if (line.trim().length > 0) { + newContentLines.push('') + } + } + } + newContentLines.push(line) + }) + + await fsp.writeFile(fullPath, newContentLines.join('\n')) + await reporter(true) + return true +} + +const oldFilterFirstCommentLines = `/** + * Instantiate object used to store the methods registered as a + * 'filter' (of the same name) within nunjucks. You can override + * gov.uk core filters by creating filter methods of the same name. + * @type {Object} + */`.split('\n').map(line => line.trim()) + +const oldFilterSecondCommentLines = `/* ------------------------------------------------------------------ + add your methods to the filters obj below this comment block: + @example: + + filters.sayHi = function(name) { + return 'Hi ' + name + '!' + } + + Which in your templates would be used as: + + {{ 'Paul' | sayHi }} => 'Hi Paul' + + Notice the first argument of your filters method is whatever + gets 'piped' via '|' to the filter. + + Filters can take additional arguments, for example: + + filters.sayHi = function(name,tone) { + return (tone == 'formal' ? 'Greetings' : 'Hi') + ' ' + name + '!' + } + + Which would be used like this: + + {{ 'Joel' | sayHi('formal') }} => 'Greetings Joel!' + {{ 'Gemma' | sayHi }} => 'Hi Gemma!' + + For more on filters and how to write them see the Nunjucks + documentation. + + ------------------------------------------------------------------ */ +`.split('\n').map(line => line.trim()) + +const oldFilterThirdCommentLines = ` + /* ------------------------------------------------------------------ + keep the following line to return your filters to the app + ------------------------------------------------------------------ */`.split('\n').map(line => line.trim()) + +async function upgradeFiltersJs (fullPath, reporter) { + const firstLine = 'module.exports = function (env) {' + const lastLine = 'return filters' + const originalContent = await fsp.readFile(fullPath, 'utf8') + + // Only convert the filters if the first and last lines above are found in the file + if (![firstLine, lastLine].every(line => originalContent.includes(line))) { + await reporter(false) + return false + } + + const matchText = [ + [firstLine], + oldFilterFirstCommentLines, + oldFilterSecondCommentLines, + oldFilterThirdCommentLines, + [lastLine] + ] + const starterFile = path.join(starterDir, 'app', 'filters.js') + const starterContent = await fsp.readFile(starterFile, 'utf8') + + // Remove the matchText from the original code + const modifiedContent = removeMatchedText(originalContent, matchText) + + // Remove the final bracket and add the addFilter conversion code + const newContent = starterContent + '\n' + + modifiedContent.substring(0, modifiedContent.lastIndexOf('}')).trimEnd() + '\n' + ` +// Add the filters using the addFilter function +Object.entries(filters).forEach(([name, fn]) => addFilter(name, fn)) +` + await fsp.writeFile(fullPath, newContent) + await reporter(true) + return true +} + +async function upgradeIfPossible (filePath, matchFound) { + if (matchFound) { + return true + } else { + const reporter = await addReporter(`Update ${filePath}`) + const fullPath = path.join(projectDir, filePath) + const filename = fullPath.split(path.sep).pop() + switch (filename) { + case 'application.js': + return await upgradeApplicationJs(fullPath, reporter) + case 'filters.js': + return await upgradeFiltersJs(fullPath, reporter) + default: + await reporter(false) + return false + } + } +} + +module.exports = { + upgradeIfPossible +} diff --git a/migrator/upgrade-steps.spec.js b/migrator/upgrade-steps.spec.js new file mode 100644 index 0000000000..bd358274a3 --- /dev/null +++ b/migrator/upgrade-steps.spec.js @@ -0,0 +1,234 @@ +/* eslint-env jest */ + +// core dependencies +const path = require('path') + +jest.mock('fs', () => { + const readFile = jest.fn().mockResolvedValue(true) + const writeFile = jest.fn().mockResolvedValue(true) + return { + promises: { + readFile, + writeFile + } + } +}) + +jest.mock('./reporter', () => { + const mockReporter = jest.fn() + return { + addReporter: jest.fn().mockReturnValue(mockReporter), + mockReporter, + reportFailure: jest.fn(), + reportSuccess: jest.fn() + } +}) + +const fsp = require('fs').promises +const reporter = require('./reporter') +const { projectDir } = require('../lib/utils/paths') +const { upgradeIfPossible } = require('./upgrade-steps') + +describe('upgrade steps', () => { + const mockReporter = reporter.mockReporter + + afterEach(() => { + jest.clearAllMocks() + }) + + it('upgrade application file if possible replacing jquery ready', async () => { + const applicationJsFile = path.join('app', 'assets', 'javascripts', 'application.js') + const matchFound = false + const originalFileContents = `/* global $ */ + +// Warn about using the kit in production +if (window.console && window.console.info) { + window.console.info('GOV.UK Prototype Kit - do not use for production') +} + +$(document).ready(function () { + window.GOVUKFrontend.initAll() + console.log('Hello') +}) +` + const expectedFileContents = `// +// For guidance on how to add JavaScript see: +// https://prototype-kit.service.gov.uk/docs/adding-css-javascript-and-images +// + +window.GOVUKPrototypeKit.documentReady(function () { + console.log('Hello') +}) +` + + fsp.readFile + .mockReturnValueOnce(originalFileContents) + + // mock call upgradeIfPossible method (get this working first) + const result = await upgradeIfPossible(applicationJsFile, matchFound) + expect(result).toBeTruthy() + const expectedFileName = path.join(projectDir, applicationJsFile) + + expect(fsp.readFile).toHaveBeenCalledTimes(1) + expect(fsp.readFile).toHaveBeenCalledWith(expectedFileName, 'utf8') + + expect(fsp.writeFile).toHaveBeenCalledTimes(1) + const [actualFileName, actualFileContent] = fsp.writeFile.mock.calls[0] + expect(actualFileName).toEqual(expectedFileName) + expect(actualFileContent).toEqual(expectedFileContents) + + expect(reporter.addReporter).toHaveBeenCalledTimes(1) + expect(reporter.addReporter).toHaveBeenCalledWith(`Update ${applicationJsFile}`) + + expect(mockReporter).toHaveBeenCalledTimes(1) + expect(mockReporter).toHaveBeenCalledWith(true) + }) + + it('upgrade application file if possible without replacing jquery ready', async () => { + const applicationJsFile = path.join('app', 'assets', 'javascripts', 'application.js') + const matchFound = false + const originalFileContents = `/* global $ */ + +// Warn about using the kit in production +if (window.console && window.console.info) { + window.console.info('GOV.UK Prototype Kit - do not use for production') +} + +$(document).ready(function () { + window.GOVUKFrontend.initAll() + $('.my-button').click(() => console.log('clicked')) +}) +` + const expectedFileContents = `/* global $ */ + +// +// For guidance on how to add JavaScript see: +// https://prototype-kit.service.gov.uk/docs/adding-css-javascript-and-images +// + + +$(document).ready(function () { + $('.my-button').click(() => console.log('clicked')) +}) +` + + fsp.readFile.mockReturnValue(originalFileContents) // For the second call + + // mock call upgradeIfPossible method (get this working first) + const result = await upgradeIfPossible(applicationJsFile, matchFound) + expect(result).toBeTruthy() + const expectedFileName = path.join(projectDir, applicationJsFile) + + expect(fsp.readFile).toHaveBeenCalledTimes(1) + expect(fsp.readFile).toHaveBeenCalledWith(expectedFileName, 'utf8') + + expect(fsp.writeFile).toHaveBeenCalledTimes(1) + const [actualFileName, actualFileContent] = fsp.writeFile.mock.calls[0] + expect(actualFileName).toEqual(expectedFileName) + expect(actualFileContent).toEqual(expectedFileContents) + + expect(reporter.addReporter).toHaveBeenCalledTimes(1) + expect(reporter.addReporter).toHaveBeenCalledWith(`Update ${applicationJsFile}`) + + expect(mockReporter).toHaveBeenCalledTimes(1) + expect(mockReporter).toHaveBeenCalledWith(true) + }) + + it('upgrade filters file if possible', async () => { + const filtersJsFile = path.join('app', 'assets', 'javascripts', 'filters.js') + const matchFound = false + const originalFileContents = `module.exports = function (env) { + var filters = {} + + /* ------------------------------------------------------------------ + add your methods to the filters obj below this comment block: + @example: + + filters.sayHi = function(name) { + return 'Hi ' + name + '!' + } + + Which in your templates would be used as: + + {{ 'Paul' | sayHi }} => 'Hi Paul' + + Notice the first argument of your filters method is whatever + gets 'piped' via '|' to the filter. + + Filters can take additional arguments, for example: + + filters.sayHi = function(name,tone) { + return (tone == 'formal' ? 'Greetings' : 'Hi') + ' ' + name + '!' + } + + Which would be used like this: + + {{ 'Joel' | sayHi('formal') }} => 'Greetings Joel!' + {{ 'Gemma' | sayHi }} => 'Hi Gemma!' + + For more on filters and how to write them see the Nunjucks + documentation. + + ------------------------------------------------------------------ */ + + function getData(){ + var dummyData = require('./data/data.js') + return dummyData; + } + + filters.toID = function (str) { + return str.replaceAll(" ", "-").toLowerCase(); + } + + /* ------------------------------------------------------------------ + keep the following line to return your filters to the app + ------------------------------------------------------------------ */ + + return filters +} +` + const starterFileContents = `const govukPrototypeKit = require('govuk-prototype-kit') +const addFilter = govukPrototypeKit.views.addFilter +` + const expectedFileContents = `const govukPrototypeKit = require('govuk-prototype-kit') +const addFilter = govukPrototypeKit.views.addFilter + + var filters = {} + + function getData(){ + var dummyData = require('./data/data.js') + return dummyData; + } + + filters.toID = function (str) { + return str.replaceAll(" ", "-").toLowerCase(); + } + +// Add the filters using the addFilter function +Object.entries(filters).forEach(([name, fn]) => addFilter(name, fn)) +` + + fsp.readFile + .mockReturnValueOnce(originalFileContents) // For the first call + .mockReturnValueOnce(starterFileContents) // For the second call + + // mock call upgradeIfPossible method (get this working first) + const result = await upgradeIfPossible(filtersJsFile, matchFound) + expect(result).toBeTruthy() + const expectedFileName = path.join(projectDir, filtersJsFile) + + expect(fsp.readFile).toHaveBeenCalledTimes(2) + expect(fsp.readFile).toHaveBeenCalledWith(expectedFileName, 'utf8') + + expect(fsp.writeFile).toHaveBeenCalledTimes(1) + const [actualFileName, actualFileContent] = fsp.writeFile.mock.calls[0] + expect(actualFileName).toEqual(expectedFileName) + expect(actualFileContent).toEqual(expectedFileContents) + + expect(reporter.addReporter).toHaveBeenCalledTimes(1) + expect(reporter.addReporter).toHaveBeenCalledWith(`Update ${filtersJsFile}`) + + expect(mockReporter).toHaveBeenCalledTimes(1) + expect(mockReporter).toHaveBeenCalledWith(true) + }) +})