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

Migrate changed files if possible #2049

Merged
merged 22 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f7d82c2
Migrate changed files if possible
BenSurgisonGDS Mar 14, 2023
66c836d
adding test for upgradeIfPossible method
HannahJMWood Mar 20, 2023
58e6336
refactoring application update into its own method
HannahJMWood Mar 21, 2023
b783ffb
started fixing issues with failing test
HannahJMWood Mar 28, 2023
c68628a
Replace any of the sections if possible
BenSurgisonGDS Mar 28, 2023
d4816f9
add globals line if there's jquery in application
HannahJMWood Mar 28, 2023
fe19a65
adding another new line and comment
HannahJMWood Mar 28, 2023
7c41308
Make sure all original lines exist in sequence within the file before…
BenSurgisonGDS Mar 28, 2023
2cb2652
addressing some review comments
HannahJMWood Mar 31, 2023
6a0618e
Just insert user's code into starter application code
BenSurgisonGDS Mar 31, 2023
abfa486
Fix verbose log
BenSurgisonGDS Apr 3, 2023
67fafe5
Simplify
BenSurgisonGDS Apr 4, 2023
c442ed0
Add change log entry
BenSurgisonGDS Apr 4, 2023
9db7e7a
Migrate changed filters if possible
BenSurgisonGDS Apr 3, 2023
3c9b1f9
Fixed tests
BenSurgisonGDS Apr 3, 2023
e70e3cb
Add change log entry
BenSurgisonGDS Apr 4, 2023
59764d4
Fix application.js conversion
BenSurgisonGDS Apr 4, 2023
c6a064c
Merge pull request #2079 from alphagov/migrate-changed-filters-if-pos…
BenSurgisonGDS Apr 4, 2023
40f2d4e
Add more complex application.js to test
BenSurgisonGDS Apr 4, 2023
e0518bc
Add line containing the frontend initAll function to make sure it is …
BenSurgisonGDS Apr 5, 2023
6a2f813
Remove big comment if unchanged from filters file and move upgrade fu…
BenSurgisonGDS Apr 5, 2023
46e3a5f
Fix test
BenSurgisonGDS Apr 5, 2023
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* global $ */
/* global GOVUK */

// Warn about using the kit in production
if (window.console && window.console.info) {
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've removed initAll() from this test, I think it's important that it's still in.

// 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()
})
33 changes: 23 additions & 10 deletions __tests__/spec/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion migrator/file-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions migrator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ const {
upgradeIfUnchanged,
deleteIfUnchanged,
removeOldPatternIncludesFromSassFile,
updateUnbrandedLayouts
updateUnbrandedLayouts,
upgradeIfPossible
} = require('./migration-steps')

const supportPage = 'https://prototype-kit.service.gov.uk/docs/support'
Expand Down Expand Up @@ -142,7 +143,7 @@ async function migrate () {
prepareSass('app/assets/sass/application.scss'),
deleteUnusedFiles(filesToDelete),
deleteUnusedDirectories(directoriesToDelete),
upgradeIfUnchanged(filesToUpdateIfUnchanged),
upgradeIfUnchanged(filesToUpdateIfUnchanged, '', upgradeIfPossible),

This comment was marked as resolved.

upgradeLayoutIfUnchanged(),
updateUnbrandedLayouts('app/views'),
deleteIfUnchanged(filesToDeleteIfUnchanged),
Expand Down
8 changes: 4 additions & 4 deletions migrator/migration-steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -254,5 +253,6 @@ module.exports = {
deleteEmptyDirectories,
deleteIfUnchanged,
upgradeIfUnchanged,
updateUnbrandedLayouts
updateUnbrandedLayouts,
upgradeIfPossible
}
11 changes: 9 additions & 2 deletions migrator/migration-steps.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})

Expand Down
1 change: 1 addition & 0 deletions migrator/migrator.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
190 changes: 190 additions & 0 deletions migrator/upgrade-steps.js
Original file line number Diff line number Diff line change
@@ -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
}
Loading