diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d115a0c25..169fcbe9e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - [#2183: Improve the design of the error screen](https://github.com/alphagov/govuk-prototype-kit/pull/2183) +### Fixes + +- [#2197: Prevent screen flickering when installing updates](https://github.com/alphagov/govuk-prototype-kit/pull/2197) + ## 13.7.1 ### Fixes diff --git a/__tests__/spec/errors.js b/__tests__/spec/errors.js index c1a872a8ca..1eedd27dc1 100644 --- a/__tests__/spec/errors.js +++ b/__tests__/spec/errors.js @@ -5,6 +5,8 @@ const request = require('supertest') const path = require('path') const cheerio = require('cheerio') +jest.mock('../../lib/sync-changes') + // local dependencies const { sleep } = require('../../lib/utils') @@ -80,7 +82,7 @@ describe('error handling', () => { expect(response.status).toBe(500) expect(getPageTitle(response.text)).toEqual('Error – GOV.UK Prototype Kit – GOV.UK Prototype Kit') expect(getH1(response.text)).toEqual('There is an error') - expect(getErrorFile(response.text)).toEqual('__tests__/spec/errors.js (line 71)') + expect(getErrorFile(response.text)).toEqual('__tests__/spec/errors.js (line 73)') expect(getErrorMessage(response.text)).toEqual('test error') app.close() diff --git a/lib/errorServer.js b/lib/errorServer.js index 781c058218..ca99104c6d 100644 --- a/lib/errorServer.js +++ b/lib/errorServer.js @@ -4,8 +4,10 @@ const fs = require('fs') const { getNunjucksAppEnv } = require('./nunjucks/nunjucksConfiguration') const errorView = require('./utils/errorView') const syncChanges = require('./sync-changes') +const { flagError } = require('./sync-changes') function runErrorServer (error) { + flagError(error) let port try { port = require('./config.js').getConfig().port diff --git a/lib/sync-changes.js b/lib/sync-changes.js index ae5c75914b..9055e6f117 100644 --- a/lib/sync-changes.js +++ b/lib/sync-changes.js @@ -3,13 +3,35 @@ const EventEmitter = require('events') // npm dependencies const browserSync = require('browser-sync') +const { writeJsonSync } = require('fs-extra') +const path = require('path') +const { tmpDir } = require('./utils/paths') +const fs = require('fs') const eventEmitter = new EventEmitter() const pageLoadedEvent = 'sync-changes:page-loaded' +const errorsFile = path.join(tmpDir, 'errors.json') + +function hasRestartedAfterError () { + return fs.existsSync(errorsFile) +} + +function flagError (error) { + writeJsonSync(path.join(tmpDir, 'errors.json'), { error }) +} + +function unflagError () { + if (hasRestartedAfterError()) { + fs.unlinkSync(errorsFile) + } +} + function pageLoaded () { - eventEmitter.emit(pageLoadedEvent) + if (hasRestartedAfterError()) { + eventEmitter.emit(pageLoadedEvent) + } return { status: 'received ok' } } @@ -26,13 +48,16 @@ function sync ({ port, proxyPort, files }) { logLevel: 'error', callbacks: { ready: (_, bs) => { - // Repeat browser sync reload every 1000 milliseconds until it is successful - const intervalId = setInterval(browserSync.reload, 1000) - eventEmitter.once(pageLoadedEvent, () => { - bs.events.once('browser:reload', () => { - clearInterval(intervalId) + if (hasRestartedAfterError()) { + // Repeat browser sync reload every 1000 milliseconds until it is successful + const intervalId = setInterval(browserSync.reload, 1000) + eventEmitter.once(pageLoadedEvent, () => { + bs.events.once('browser:reload', () => { + unflagError() + clearInterval(intervalId) + }) }) - }) + } } } }) @@ -40,5 +65,6 @@ function sync ({ port, proxyPort, files }) { module.exports = { sync, + flagError, pageLoaded } diff --git a/lib/sync-changes.test.js b/lib/sync-changes.test.js new file mode 100644 index 0000000000..a3f4cb8c94 --- /dev/null +++ b/lib/sync-changes.test.js @@ -0,0 +1,67 @@ +const syncChanges = require('./sync-changes') + +jest.mock('fs') +jest.mock('fs-extra') +jest.mock('browser-sync') + +const fs = require('fs') +const fse = require('fs-extra') +const browserSync = require('browser-sync') +const path = require('path') +const { tmpDir } = require('./utils/paths') + +const errorsFile = path.join(tmpDir, 'errors.json') + +describe('sync-changes', () => { + beforeEach(() => { + // + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it('flags error correctly', () => { + const error = { data: true } + + syncChanges.flagError(error) + + expect(fse.writeJsonSync).toHaveBeenCalledTimes(1) + expect(fse.writeJsonSync).toHaveBeenCalledWith(errorsFile, { error }) + }) + + it('syncs correctly', () => { + const port = 1000 + const proxyPort = 900 + const files = ['test-file'] + + syncChanges.sync({ port, proxyPort, files }) + + const browserSyncParams = browserSync.mock.calls[0][0] + + expect(browserSyncParams).toHaveProperty('port', port) + expect(browserSyncParams).toHaveProperty('proxy', `localhost:${900}`) + expect(browserSyncParams).toHaveProperty('port', port) + + jest.spyOn(fs, 'existsSync').mockImplementation(() => { + return true + }) + + const once = (event, callback) => { + expect(event).toEqual('browser:reload') + callback() + } + + const bs = { events: { once } } + + browserSyncParams.callbacks.ready(null, bs) + + syncChanges.pageLoaded() + + expect(fs.existsSync).toHaveBeenCalledTimes(3) + expect(fs.existsSync).toHaveBeenCalledWith(errorsFile) + + expect(fs.unlinkSync).toHaveBeenCalledTimes(1) + expect(fs.unlinkSync).toHaveBeenCalledWith(errorsFile) + }) +}) diff --git a/lib/utils/errorView.js b/lib/utils/errorView.js index 98cd5c557e..ffefdc4573 100644 --- a/lib/utils/errorView.js +++ b/lib/utils/errorView.js @@ -1,6 +1,7 @@ const fs = require('fs') const path = require('path') const { verboseLog } = require('./verboseLogger') +const { flagError } = require('../sync-changes') function getSourcecode (filePath, errorLineNumber, maxLines) { try { @@ -111,6 +112,8 @@ function nodeMatchWithLineAndColumn (error) { } module.exports = function (error) { + flagError(error) + const viewObject = { errorStack: error.stack, pluginConfig: {