From ce64c022799bfdf117da0899d68feaca236a819a Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 22 Jul 2022 14:24:53 +0100 Subject: [PATCH 1/8] Move shared JavaScript to lib folder --- __tests__/spec/build.js | 2 +- {app => lib}/assets/javascripts/application.js | 0 {app => lib}/assets/javascripts/auto-store-data.js | 0 lib/build/tasks.js | 4 +++- 4 files changed, 4 insertions(+), 2 deletions(-) rename {app => lib}/assets/javascripts/application.js (100%) rename {app => lib}/assets/javascripts/auto-store-data.js (100%) diff --git a/__tests__/spec/build.js b/__tests__/spec/build.js index da74055ae2..1000957dba 100644 --- a/__tests__/spec/build.js +++ b/__tests__/spec/build.js @@ -86,7 +86,7 @@ describe('the build pipeline', () => { it('copies javascript to the public folder', () => { expect(fs.copyFileSync).toHaveBeenCalledWith( - path.join('app', 'assets', 'javascripts', 'application.js'), + path.join(projectDir, 'lib', 'assets', 'javascripts', 'application.js'), path.join('public', 'javascripts', 'application.js') ) }) diff --git a/app/assets/javascripts/application.js b/lib/assets/javascripts/application.js similarity index 100% rename from app/assets/javascripts/application.js rename to lib/assets/javascripts/application.js diff --git a/app/assets/javascripts/auto-store-data.js b/lib/assets/javascripts/auto-store-data.js similarity index 100% rename from app/assets/javascripts/auto-store-data.js rename to lib/assets/javascripts/auto-store-data.js diff --git a/lib/build/tasks.js b/lib/build/tasks.js index 410aaff022..e9c56f0783 100644 --- a/lib/build/tasks.js +++ b/lib/build/tasks.js @@ -16,7 +16,8 @@ const { projectDir, packageDir } = require('../path-utils') const { paths } = buildConfig const appSassPath = path.join(projectDir, paths.assets, 'sass') -const libSassPath = path.join(packageDir, paths.libAssets, 'sass') +const libAssetsPath = path.join(packageDir, paths.libAssets) +const libSassPath = path.join(libAssetsPath, 'sass') const tempPath = path.join(projectDir, '.tmp') const tempSassPath = path.join(tempPath, 'sass') @@ -46,6 +47,7 @@ function generateAssetsSync ({ verbose } = {}) { if (verbose) process.stdout.write('done\n') if (verbose) process.stdout.write('copying assets...') + copyAssets(libAssetsPath, paths.public) copyAssets(paths.assets, paths.public) if (verbose) process.stdout.write('done\n') } From 07c748b3f0d185aba27f98ee7c273311ab7f3789 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 22 Jul 2022 14:32:14 +0100 Subject: [PATCH 2/8] Add empty app/.../application.js for users to add their own JavaScript Also rename application.js to kit.js to avoid clashes. Co-authored-by: Joe Lanman --- __tests__/spec/build.js | 7 ++++++- app/assets/javascripts/application.js | 3 +++ app/views/includes/scripts.html | 1 + .../integration/1-watch-files/watch-javascripts.cypress.js | 2 +- lib/assets/javascripts/{application.js => kit.js} | 0 5 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/application.js rename lib/assets/javascripts/{application.js => kit.js} (100%) diff --git a/__tests__/spec/build.js b/__tests__/spec/build.js index 1000957dba..75a48f3ad6 100644 --- a/__tests__/spec/build.js +++ b/__tests__/spec/build.js @@ -86,7 +86,12 @@ describe('the build pipeline', () => { it('copies javascript to the public folder', () => { expect(fs.copyFileSync).toHaveBeenCalledWith( - path.join(projectDir, 'lib', 'assets', 'javascripts', 'application.js'), + path.join(projectDir, 'lib', 'assets', 'javascripts', 'kit.js'), + path.join('public', 'javascripts', 'kit.js') + ) + + expect(fs.copyFileSync).toHaveBeenCalledWith( + path.join('app', 'assets', 'javascripts', 'application.js'), path.join('public', 'javascripts', 'application.js') ) }) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js new file mode 100644 index 0000000000..277b4f5578 --- /dev/null +++ b/app/assets/javascripts/application.js @@ -0,0 +1,3 @@ +window.GOVUKPrototypeKit.documentReady(() => { + // Add JavaScript here +}) diff --git a/app/views/includes/scripts.html b/app/views/includes/scripts.html index 40960cd667..f61a653543 100644 --- a/app/views/includes/scripts.html +++ b/app/views/includes/scripts.html @@ -3,6 +3,7 @@ {% endfor %} + {% if useAutoStoreData %} diff --git a/cypress/integration/1-watch-files/watch-javascripts.cypress.js b/cypress/integration/1-watch-files/watch-javascripts.cypress.js index b9fa1293da..30b0406da1 100644 --- a/cypress/integration/1-watch-files/watch-javascripts.cypress.js +++ b/cypress/integration/1-watch-files/watch-javascripts.cypress.js @@ -22,7 +22,7 @@ describe('watch application.js', () => { const onAlert = cy.stub() cy.on('window:alert', onAlert) - const markerText = 'window.GOVUKFrontend.initAll()' + const markerText = '// Add JavaScript here' const newText = markerText + '\n ' + "window.alert('Test')" cy.task('replaceTextInFile', { diff --git a/lib/assets/javascripts/application.js b/lib/assets/javascripts/kit.js similarity index 100% rename from lib/assets/javascripts/application.js rename to lib/assets/javascripts/kit.js From 9d8dc486fd1726575b91bc267ad78ba18571da6d Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 22 Jul 2022 16:19:32 +0100 Subject: [PATCH 3/8] Add update post script to update user's app javascripts Co-authored-by: NoraGDS <57447099+NoraGDS@users.noreply.github.com> --- __tests__/spec/update-script.js | 17 +++ lib/_update/update-javascripts.js | 34 ++++++ lib/_update/update-javascripts.test.js | 124 ++++++++++++++++++++ lib/_update/util/fetch-original.js | 33 ++++++ lib/_update/util/fetch-original.test.js | 29 +++++ lib/_update/util/index.js | 49 ++++++++ lib/_update/util/index.test.js | 144 ++++++++++++++++++++++++ update.sh | 10 +- 8 files changed, 436 insertions(+), 4 deletions(-) create mode 100644 lib/_update/update-javascripts.js create mode 100644 lib/_update/update-javascripts.test.js create mode 100644 lib/_update/util/fetch-original.js create mode 100644 lib/_update/util/fetch-original.test.js create mode 100644 lib/_update/util/index.js create mode 100644 lib/_update/util/index.test.js diff --git a/__tests__/spec/update-script.js b/__tests__/spec/update-script.js index c173a52940..b5e8fe0eb9 100644 --- a/__tests__/spec/update-script.js +++ b/__tests__/spec/update-script.js @@ -511,6 +511,23 @@ describe('update.sh', () => { }) }) + describe('post', () => { + it('updates app javascripts', async () => { + const testDir = await mktestPrototype( + 'updates-app-javascripts', { ref: 'v12.1.1' } + ) + + await runScriptAndExpectSuccess('post', { testDir }) + + expect(await execGitStatus(testDir)).toEqual(expect.arrayContaining([ + ' M app/assets/javascripts/application.js', + ' D app/assets/javascripts/auto-store-data.js', + ' D app/assets/javascripts/jquery-1.11.3.js', + ' D app/assets/javascripts/step-by-step-nav.js' + ])) + }) + }) + it('can be run as a piped script', async () => { const testDir = await mktestPrototype('pipe') diff --git a/lib/_update/update-javascripts.js b/lib/_update/update-javascripts.js new file mode 100644 index 0000000000..8c0305e689 --- /dev/null +++ b/lib/_update/update-javascripts.js @@ -0,0 +1,34 @@ +const fs = require('fs').promises +const path = require('path') + +const { getProjectVersion, patchUserFile } = require('./util') +const { projectDir } = require('../path-utils') + +async function updateJavascripts () { + // Delete any old shared files + const appJsPath = path.join(projectDir, 'app', 'assets', 'javascripts') + await fs.unlink(path.join(appJsPath, 'auto-store-data.js')).catch(() => {}) + await fs.unlink(path.join(appJsPath, 'jquery-1.11.3.js')).catch(() => {}) + await fs.unlink(path.join(appJsPath, 'step-by-step-nav.js')).catch(() => {}) + await fs.unlink(path.join(appJsPath, 'step-by-step-navigation.js')).catch(() => {}) + + const userVersion = await getProjectVersion() + + // If the user already has version 13 or greater of the kit installed then + // their application.js file is all their code and we don't don't want to + // change it + if (userVersion >= '13.0.0') { + return + } + + await patchUserFile(userVersion, 'app/assets/javascripts/application.js') +} + +module.exports = { + /* exported for tests only */ + updateJavascripts +} + +if (require.main === module) { + updateJavascripts() +} diff --git a/lib/_update/update-javascripts.test.js b/lib/_update/update-javascripts.test.js new file mode 100644 index 0000000000..3658d17699 --- /dev/null +++ b/lib/_update/update-javascripts.test.js @@ -0,0 +1,124 @@ +/* eslint-env jest */ + +const fs = require('fs') +const fsp = require('fs').promises +const path = require('path') + +jest.mock('./util/fetch-original') +jest.mock('./util', () => { + const originalModule = jest.requireActual('./util') + + return { + ...originalModule, + getProjectVersion: jest.fn(async () => '') + } +}) +const { fetchOriginal: mockFetchOriginal } = require('./util/fetch-original') +const { getProjectVersion: mockGetProjectVersion } = require('./util') +const { projectDir } = require('../path-utils') + +const { updateJavascripts } = require('./update-javascripts') + +const originalApplicationJs = `/* 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() +}) +` +const newApplicationJs = fs.readFileSync( + path.join('app', 'assets', 'javascripts', 'application.js'), + 'utf8' +) + +describe('updateJavascripts', () => { + let mockCopyFile, mockReadFile, mockUnlink, mockWriteFile + + beforeEach(async () => { + mockGetProjectVersion.mockResolvedValue( + '12.1.1' + ) + + mockFetchOriginal.mockResolvedValue( + originalApplicationJs + ) + + mockReadFile = jest.spyOn(fsp, 'readFile').mockResolvedValue( + newApplicationJs + ) + + mockCopyFile = jest.spyOn(fsp, 'copyFile').mockImplementation(async () => {}) + mockUnlink = jest.spyOn(fsp, 'unlink').mockImplementation(async () => {}) + mockWriteFile = jest.spyOn(fsp, 'writeFile').mockImplementation(async () => {}) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it('replaces application.js if the user has not updated it', async () => { + // theirs + mockReadFile.mockResolvedValueOnce( + originalApplicationJs + ) + + await updateJavascripts() + + expect(mockCopyFile).toHaveBeenCalledWith( + path.join(projectDir, 'update', 'app', 'assets', 'javascripts', 'application.js'), + path.join(projectDir, 'app', 'assets', 'javascripts', 'application.js') + ) + }) + + it('rewrites application.js if the user has added lines to the bottom of the file', async () => { + // theirs + mockReadFile.mockResolvedValueOnce( + originalApplicationJs + '\ncallMyCode()\n' + ) + // ours + mockReadFile.mockResolvedValue( + newApplicationJs + ) + + await updateJavascripts() + + expect(mockWriteFile).toHaveBeenCalledWith( + path.join(projectDir, 'app', 'assets', 'javascripts', 'application.js'), + newApplicationJs + '\ncallMyCode()\n', + 'utf8' + ) + }) + + it('does not touch application.js if the user prototype is already on v13', async () => { + mockGetProjectVersion.mockResolvedValue( + '13.0.0' + ) + + await updateJavascripts() + + expect(mockReadFile).not.toHaveBeenCalled() + expect(mockWriteFile).not.toHaveBeenCalled() + expect(mockCopyFile).not.toHaveBeenCalled() + }) + + it('removes files that have been moved from app folder', async () => { + await updateJavascripts() + + expect(mockUnlink).toHaveBeenCalledWith( + path.join(projectDir, 'app', 'assets', 'javascripts', 'auto-store-data.js') + ) + expect(mockUnlink).toHaveBeenCalledWith( + path.join(projectDir, 'app', 'assets', 'javascripts', 'jquery-1.11.3.js') + ) + expect(mockUnlink).toHaveBeenCalledWith( + path.join(projectDir, 'app', 'assets', 'javascripts', 'step-by-step-nav.js') + ) + expect(mockUnlink).toHaveBeenCalledWith( + path.join(projectDir, 'app', 'assets', 'javascripts', 'step-by-step-navigation.js') + ) + }) +}) diff --git a/lib/_update/util/fetch-original.js b/lib/_update/util/fetch-original.js new file mode 100644 index 0000000000..81a5ea1115 --- /dev/null +++ b/lib/_update/util/fetch-original.js @@ -0,0 +1,33 @@ +const https = require('https') + +async function fetchOriginal (version, filePath) { + const remoteUrl = `https://raw.githubusercontent.com/alphagov/govuk-prototype-kit/v${version}/${filePath}` + + let data = '' + return new Promise((resolve, reject) => { + https.get(remoteUrl, (response) => { + let error + + if (response.statusCode !== 200) { + error = new Error(`could not fetch ${remoteUrl}: status code ${response.statusCode}`) + Object.assign(error, response) + response.resume() + reject(error) + } + + response.setEncoding('utf8') + + response.on('data', (chunk) => { + data += chunk + }) + + response.on('end', () => { + resolve(data) + }) + }) + }) +} + +module.exports = { + fetchOriginal +} diff --git a/lib/_update/util/fetch-original.test.js b/lib/_update/util/fetch-original.test.js new file mode 100644 index 0000000000..1f4fe76011 --- /dev/null +++ b/lib/_update/util/fetch-original.test.js @@ -0,0 +1,29 @@ +/* eslint-env jest */ + +const https = require('https') +const stream = require('stream') + +const { fetchOriginal } = require('./fetch-original') + +describe('fetchOriginal', () => { + it('gets the contents of a file as of version from GitHub', async () => { + const mockHttpsGet = jest.spyOn(https, 'get').mockImplementation((url, callback) => { + const response = new stream.PassThrough() + response.statusCode = 200 + + callback(response) + + response.write('foo\n') + response.write('bar\n') + response.end() + }) + + await expect(fetchOriginal('99.99.99', 'app/views/foo.html')).resolves.toEqual( + 'foo\nbar\n' + ) + expect(mockHttpsGet).toHaveBeenCalledWith( + 'https://raw.githubusercontent.com/alphagov/govuk-prototype-kit/v99.99.99/app/views/foo.html', + expect.anything() + ) + }) +}) diff --git a/lib/_update/util/index.js b/lib/_update/util/index.js new file mode 100644 index 0000000000..9aca0ba0cd --- /dev/null +++ b/lib/_update/util/index.js @@ -0,0 +1,49 @@ +const fs = require('fs').promises +const path = require('path') + +const { projectDir } = require('../../path-utils') +const { fetchOriginal } = require('./fetch-original') + +const updateDir = path.join(projectDir, 'update') + +async function getProjectVersion () { + return (await fs.readFile(path.join(projectDir, 'VERSION.txt'), 'utf8')).trim() +} + +async function patchUserFile (originalVersion, filePath) { + const original = await fetchOriginal(originalVersion, filePath) + const theirs = await fs.readFile(path.resolve(projectDir, filePath), 'utf8') + const ours = await fs.readFile(path.resolve(updateDir, filePath), 'utf8') + + // It is possible that the file has already been upgraded, in which case there is nothing to do + if (theirs === ours) { + return + } + + // If the user hasn't changed the file we can just replace it completely + if (original === theirs) { + return fs.copyFile(path.join(updateDir, filePath), path.join(projectDir, filePath)) + } + + // Otherwise, if the original code is contained as-is in their file, we can + // remove the shared lines, and add our hints + if (theirs.includes(original)) { + let merged + merged = theirs.replace(original, '') + merged = ours + merged + return fs.writeFile(path.resolve(projectDir, filePath), merged, 'utf8') + } + + // If the original code is not recognisable, we should give up, but not + // without giving a warning to the user + console.warn( + `WARNING: update.sh was not able to automatically update your ${filePath} file.\n` + + 'If you have a problem when running your prototype contact the GOV.UK Prototype team for support,\n' + + 'using one of the methods listed at https://design-system.service.gov.uk/get-in-touch/' + ) +} + +module.exports = { + getProjectVersion, + patchUserFile +} diff --git a/lib/_update/util/index.test.js b/lib/_update/util/index.test.js new file mode 100644 index 0000000000..a51924642a --- /dev/null +++ b/lib/_update/util/index.test.js @@ -0,0 +1,144 @@ +/* eslint-env jest */ + +const fs = require('fs').promises +const path = require('path') + +jest.mock('./fetch-original') +const { fetchOriginal: mockFetchOriginal } = require('./fetch-original') +const { projectDir } = require('../../path-utils') + +const { getProjectVersion, patchUserFile } = require('./index.js') + +afterEach(() => { + jest.restoreAllMocks() +}) + +describe('getProjectVersion', () => { + it('reads the VERSION.txt file to get the version number', async () => { + const mockReadFile = jest.spyOn(fs, 'readFile').mockImplementation( + async () => '99.99.99\n' + ) + + await expect(getProjectVersion()).resolves.toEqual('99.99.99') + expect(mockReadFile).toHaveBeenCalledWith( + expect.stringContaining(`${path.sep}VERSION.txt`), + 'utf8' + ) + }) +}) + +describe('patchUserFile', () => { + const filePath = 'app/assets/javascripts/application.js' + + const originalContents = `/* global $ */ +$(document).ready(function () { + window.GOVUKFrontend.initAll() +}) +` + + const newContents = `window.GOVUKPrototypeKit.ready(() => { + // Add JavaScript here +}) +` + + let mockCopyFile, mockReadFile, mockWriteFile + + beforeEach(() => { + mockFetchOriginal.mockResolvedValue( + originalContents + ) + + mockReadFile = jest.spyOn(fs, 'readFile').mockResolvedValue( + newContents + ) + + mockCopyFile = jest.spyOn(fs, 'copyFile').mockImplementation(() => {}) + mockWriteFile = jest.spyOn(fs, 'writeFile').mockImplementation(() => {}) + }) + + it('gets the original contents to compare with the user file', async () => { + mockReadFile.mockResolvedValue( + originalContents + ) + + mockFetchOriginal.mockResolvedValue( + originalContents + ) + + await patchUserFile('99.99.99', filePath) + + expect(mockFetchOriginal).toHaveBeenCalledWith( + '99.99.99', filePath + ) + }) + + it('replaces file if the user has not updated it', async () => { + mockReadFile.mockResolvedValueOnce( + originalContents + ) + + await patchUserFile('99.99.99', filePath) + + expect(mockCopyFile).toHaveBeenCalledWith( + path.join(projectDir, 'update', 'app', 'assets', 'javascripts', 'application.js'), + path.join(projectDir, 'app', 'assets', 'javascripts', 'application.js') + ) + }) + + it('rewrites file if the user has added lines to the bottom of the file', async () => { + // theirs + jest.spyOn(fs, 'readFile').mockResolvedValueOnce( + originalContents + '\ncallMyCode()\n' + ) + // ours + jest.spyOn(fs, 'readFile').mockResolvedValueOnce( + newContents + ) + + await patchUserFile('99.99.99', filePath) + + expect(mockWriteFile).toHaveBeenCalledWith( + expect.stringContaining(path.join('app', 'assets', 'javascripts', 'application.js')), + newContents + '\ncallMyCode()\n', + 'utf8' + ) + }) + + it('warns user and does not touch file if the user has rewritten it a lot', async () => { + // theirs + jest.spyOn(fs, 'readFile').mockImplementationOnce( + async () => 'justMyCode()\n' + ) + // ours + jest.spyOn(fs, 'readFile').mockResolvedValueOnce( + newContents + ) + + const mockConsoleWarn = jest.spyOn(global.console, 'warn').mockImplementation(() => {}) + + await patchUserFile('99.99.99', filePath) + + expect(mockWriteFile).not.toHaveBeenCalled() + expect(mockCopyFile).not.toHaveBeenCalled() + expect(mockConsoleWarn).toHaveBeenCalled() + }) + + it('does nothing if file has already been updated somehow', async () => { + // theirs + jest.spyOn(fs, 'readFile').mockResolvedValueOnce( + newContents + ) + // ours + jest.spyOn(fs, 'readFile').mockResolvedValueOnce( + newContents + ) + + const mockConsoleWarn = jest.spyOn(global.console, 'warn') + + await patchUserFile('99.99.99', filePath) + + expect(mockWriteFile).not.toHaveBeenCalled() + expect(mockCopyFile).not.toHaveBeenCalled() + expect(mockConsoleWarn).not.toHaveBeenCalled() + }) +}) diff --git a/update.sh b/update.sh index 2af30c4725..162c784d6a 100755 --- a/update.sh +++ b/update.sh @@ -181,10 +181,12 @@ copy () { } post () { - # execute _update_scss if it exists in the update folder - if [ -d "update/lib/_update_scss" ]; then - node "update/lib/_update_scss" - fi + if [ -d "update/lib/_update_scss" ]; then + node "update/lib/_update_scss" + fi + if [ -f "update/lib/_update/update-javascripts.js" ]; then + node "update/lib/_update/update-javascripts" + fi } if [ "$0" == "${BASH_SOURCE:-$0}" ] From 63d1550dd3d8d99fc21a64fa8ead32d4a4a08d53 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 28 Jul 2022 14:13:46 +0100 Subject: [PATCH 4/8] Copy kit javascripts to private namespace in public folder We want to make sure that any shared JS files we create can't have name collisions with files the user wants to create. Putting the files in a separate namespaced folder mitigates against this. --- __tests__/spec/build.js | 2 +- app/views/includes/scripts.html | 4 ++-- lib/build/tasks.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/__tests__/spec/build.js b/__tests__/spec/build.js index 75a48f3ad6..b65abee332 100644 --- a/__tests__/spec/build.js +++ b/__tests__/spec/build.js @@ -87,7 +87,7 @@ describe('the build pipeline', () => { it('copies javascript to the public folder', () => { expect(fs.copyFileSync).toHaveBeenCalledWith( path.join(projectDir, 'lib', 'assets', 'javascripts', 'kit.js'), - path.join('public', 'javascripts', 'kit.js') + path.join('public', '_kit', 'javascripts', 'kit.js') ) expect(fs.copyFileSync).toHaveBeenCalledWith( diff --git a/app/views/includes/scripts.html b/app/views/includes/scripts.html index f61a653543..b078c8efb8 100644 --- a/app/views/includes/scripts.html +++ b/app/views/includes/scripts.html @@ -3,9 +3,9 @@ {% endfor %} - + {% if useAutoStoreData %} - + {% endif %} diff --git a/lib/build/tasks.js b/lib/build/tasks.js index e9c56f0783..b8d60f99ae 100644 --- a/lib/build/tasks.js +++ b/lib/build/tasks.js @@ -47,7 +47,7 @@ function generateAssetsSync ({ verbose } = {}) { if (verbose) process.stdout.write('done\n') if (verbose) process.stdout.write('copying assets...') - copyAssets(libAssetsPath, paths.public) + copyAssets(libAssetsPath, path.join(paths.public, '_kit')) copyAssets(paths.assets, paths.public) if (verbose) process.stdout.write('done\n') } From 4c023cc5d22677e2b1a1adb04208f38464b8d13f Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 2 Aug 2022 15:53:40 +0100 Subject: [PATCH 5/8] Increase test timeout The update script tests run create-release-archive a lot, which involves a lot of shelling out, and on Windows this is very slow, so we increase the test timeout again to account for this. Ideally there would be less shelling out, but that's a problem for another day. --- __tests__/spec/update-script.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/spec/update-script.js b/__tests__/spec/update-script.js index b5e8fe0eb9..063db1a1d1 100644 --- a/__tests__/spec/update-script.js +++ b/__tests__/spec/update-script.js @@ -14,7 +14,7 @@ const execPromise = promisify(child_process.exec) const execFilePromise = promisify(child_process.execFile) // This is a long-running test -jest.setTimeout(60000) +jest.setTimeout(120000) function testSkipFailingIf (condition, ...args) { if (condition) { From 5fa558f323a2f053ed310f6910ec0499c2113098 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 2 Aug 2022 16:14:23 +0100 Subject: [PATCH 6/8] Normalise line endings when patching user files We want to make sure that on Windows we can compare files regardless of what line endings the users files have, and that any changes we make have the same line endings. --- lib/_update/util/index.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/_update/util/index.js b/lib/_update/util/index.js index 9aca0ba0cd..b6493bb784 100644 --- a/lib/_update/util/index.js +++ b/lib/_update/util/index.js @@ -11,9 +11,21 @@ async function getProjectVersion () { } async function patchUserFile (originalVersion, filePath) { - const original = await fetchOriginal(originalVersion, filePath) const theirs = await fs.readFile(path.resolve(projectDir, filePath), 'utf8') - const ours = await fs.readFile(path.resolve(updateDir, filePath), 'utf8') + let original = await fetchOriginal(originalVersion, filePath) + let ours = await fs.readFile(path.resolve(updateDir, filePath), 'utf8') + + // Normalise line endings to match their file + var eol = '\n' + if (theirs.includes('\r\n')) { + eol = '\r\n' + if (!original.includes(eol)) { + original = original.replace(/\n/g, eol) + } + if (!ours.includes(eol)) { + ours = ours.replace(/\n/g, eol) + } + } // It is possible that the file has already been upgraded, in which case there is nothing to do if (theirs === ours) { From 20891d4a6978f919927925399283ee6bae949254 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 2 Aug 2022 16:41:21 +0100 Subject: [PATCH 7/8] Move _update_scss script to lib/_update --- __tests__/spec/update-script.js | 22 +++++++++++++++---- .../update-scss}/index.js | 0 .../update-scss}/update_scss.js | 2 +- .../update-scss}/update_scss.test.js | 2 +- update.sh | 4 ++-- 5 files changed, 22 insertions(+), 8 deletions(-) rename lib/{_update_scss => _update/update-scss}/index.js (100%) rename lib/{_update_scss => _update/update-scss}/update_scss.js (97%) rename lib/{_update_scss => _update/update-scss}/update_scss.test.js (97%) diff --git a/__tests__/spec/update-script.js b/__tests__/spec/update-script.js index 063db1a1d1..9a881d28e7 100644 --- a/__tests__/spec/update-script.js +++ b/__tests__/spec/update-script.js @@ -512,14 +512,28 @@ describe('update.sh', () => { }) describe('post', () => { - it('updates app javascripts', async () => { - const testDir = await mktestPrototype( - 'updates-app-javascripts', { ref: 'v12.1.1' } + let testDir, gitStatus + + beforeAll(async () => { + testDir = await mktestPrototype( + 'update-post', { ref: 'v12.1.1' } ) await runScriptAndExpectSuccess('post', { testDir }) - expect(await execGitStatus(testDir)).toEqual(expect.arrayContaining([ + gitStatus = await execGitStatus(testDir) + }) + + it('updates app stylesheets', () => { + expect(gitStatus).toEqual(expect.arrayContaining([ + ' M app/assets/sass/application.scss', + ' D app/assets/sass/application-ie8.scss', + ' D app/assets/sass/unbranded-ie8.scss' + ])) + }) + + it('updates app javascripts', () => { + expect(gitStatus).toEqual(expect.arrayContaining([ ' M app/assets/javascripts/application.js', ' D app/assets/javascripts/auto-store-data.js', ' D app/assets/javascripts/jquery-1.11.3.js', diff --git a/lib/_update_scss/index.js b/lib/_update/update-scss/index.js similarity index 100% rename from lib/_update_scss/index.js rename to lib/_update/update-scss/index.js diff --git a/lib/_update_scss/update_scss.js b/lib/_update/update-scss/update_scss.js similarity index 97% rename from lib/_update_scss/update_scss.js rename to lib/_update/update-scss/update_scss.js index 16d1c3fcbe..e179518b3a 100644 --- a/lib/_update_scss/update_scss.js +++ b/lib/_update/update-scss/update_scss.js @@ -1,6 +1,6 @@ const path = require('path') const fs = require('fs') -const { projectDir, packageDir } = require('../path-utils') +const { projectDir, packageDir } = require('../../path-utils') const appSassPath = path.join(projectDir, 'app', 'assets', 'sass') const appSassPatternsPath = path.join(appSassPath, 'patterns') const applicationScssPath = path.join(appSassPath, 'application.scss') diff --git a/lib/_update_scss/update_scss.test.js b/lib/_update/update-scss/update_scss.test.js similarity index 97% rename from lib/_update_scss/update_scss.test.js rename to lib/_update/update-scss/update_scss.test.js index 12f67f4c62..eec65daa42 100644 --- a/lib/_update_scss/update_scss.test.js +++ b/lib/_update/update-scss/update_scss.test.js @@ -4,7 +4,7 @@ const fs = require('fs') const path = require('path') const updateKit = require('./update_scss') const { appSassPath } = require('./update_scss') -const { projectDir } = require('../path-utils') +const { projectDir } = require('../../path-utils') describe('scripts/update-kit', () => { afterEach(() => { diff --git a/update.sh b/update.sh index 162c784d6a..45c9c1e5cd 100755 --- a/update.sh +++ b/update.sh @@ -181,8 +181,8 @@ copy () { } post () { - if [ -d "update/lib/_update_scss" ]; then - node "update/lib/_update_scss" + if [ -d "update/lib/_update/update-scss" ]; then + node "update/lib/_update/update-scss" fi if [ -f "update/lib/_update/update-javascripts.js" ]; then node "update/lib/_update/update-javascripts" From 5e94bc75361b735a3fb8ba577b1f6cb10f67d237 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 2 Aug 2022 17:15:40 +0100 Subject: [PATCH 8/8] Move update post scripts into one script called post Simplify the update shell script by combining the Node.js post scripts into one mega script. We also move the final message from update script to very end. --- lib/_update/post.js | 5 +++++ lib/_update/update-javascripts.js | 5 ----- lib/_update/update-scss/index.js | 10 ++++++---- update.sh | 20 +++++++++++--------- 4 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 lib/_update/post.js diff --git a/lib/_update/post.js b/lib/_update/post.js new file mode 100644 index 0000000000..521e06803f --- /dev/null +++ b/lib/_update/post.js @@ -0,0 +1,5 @@ +const { updateJavascripts } = require('./update-javascripts') +const { updateScss } = require('./update-scss') + +updateJavascripts() +updateScss() diff --git a/lib/_update/update-javascripts.js b/lib/_update/update-javascripts.js index 8c0305e689..dc222304c9 100644 --- a/lib/_update/update-javascripts.js +++ b/lib/_update/update-javascripts.js @@ -25,10 +25,5 @@ async function updateJavascripts () { } module.exports = { - /* exported for tests only */ updateJavascripts } - -if (require.main === module) { - updateJavascripts() -} diff --git a/lib/_update/update-scss/index.js b/lib/_update/update-scss/index.js index 460f625727..59e7579d66 100644 --- a/lib/_update/update-scss/index.js +++ b/lib/_update/update-scss/index.js @@ -8,7 +8,9 @@ const { removeLegacyIE8Sass } = require('./update_scss') -removeKitSassFromApplicationSass() -removeKitSassFromAppSassPath(appSassPatternsPath, libSassPatternsPath) -removeKitSassFromAppSassPath(appSassPath, libSassPath) -removeLegacyIE8Sass() +module.exports.updateScss = function () { + removeKitSassFromApplicationSass() + removeKitSassFromAppSassPath(appSassPatternsPath, libSassPatternsPath) + removeKitSassFromAppSassPath(appSassPath, libSassPath) + removeLegacyIE8Sass() +} diff --git a/update.sh b/update.sh index 45c9c1e5cd..ee94decbfd 100755 --- a/update.sh +++ b/update.sh @@ -170,6 +170,16 @@ copy () { trap - ERR + update_gitignore +} + +post () { + if [ -f 'update/lib/_update/post.js' ]; then + node 'update/lib/_update/post' + fi +} + +finish () { msg msg "Your prototype kit files have now been updated, from version ${OLD_VERSION} to ${NEW_VERSION}." msg 'If you need to make configuration changes, follow the steps at' @@ -180,15 +190,6 @@ copy () { update_gitignore } -post () { - if [ -d "update/lib/_update/update-scss" ]; then - node "update/lib/_update/update-scss" - fi - if [ -f "update/lib/_update/update-javascripts.js" ]; then - node "update/lib/_update/update-javascripts" - fi -} - if [ "$0" == "${BASH_SOURCE:-$0}" ] then check @@ -197,4 +198,5 @@ then extract copy post + finish fi