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 19 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,10 +1,22 @@
/* global $ */
/* global GOVUK */

// 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()
// 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
147 changes: 142 additions & 5 deletions migrator/migration-steps.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@

// core dependencies
const path = require('path')
const fsp = require('fs').promises

// npm dependencies
const fse = require('fs-extra')
const lodash = require('lodash')

// local dependencies
const { searchAndReplaceFiles } = require('../lib/utils')
const { appDir, projectDir, packageDir } = require('../lib/utils/paths')
const { appDir, projectDir, packageDir, starterDir } = require('../lib/utils/paths')
const config = require('../lib/config')
const logger = require('./logger')
const { addReporter, reportSuccess, reportFailure } = require('./reporter')
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 All @@ -242,6 +241,143 @@ async function updateUnbrandedLayouts (dir) {
return results.flat()
}

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
}
}
}

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
}

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],
[
'/* ------------------------------------------------------------------',
'keep the following line to return your filters to the app',
'------------------------------------------------------------------ */'
],
[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
}

module.exports = {
getOldConfig,
preflightChecks,
Expand All @@ -254,5 +390,6 @@ module.exports = {
deleteEmptyDirectories,
deleteIfUnchanged,
upgradeIfUnchanged,
updateUnbrandedLayouts
updateUnbrandedLayouts,
upgradeIfPossible
}
Loading