Skip to content

Commit

Permalink
Prevent screen flickering when installing plugins updates
Browse files Browse the repository at this point in the history
  • Loading branch information
BenSurgisonGDS committed May 31, 2023
1 parent c461ca5 commit e5cc521
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion __tests__/spec/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions lib/errorServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 33 additions & 7 deletions lib/sync-changes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
}

Expand All @@ -26,19 +48,23 @@ 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)
})
})
})
}
}
}
})
}

module.exports = {
sync,
flagError,
pageLoaded
}
67 changes: 67 additions & 0 deletions lib/sync-changes.test.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
3 changes: 3 additions & 0 deletions lib/utils/errorView.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -111,6 +112,8 @@ function nodeMatchWithLineAndColumn (error) {
}

module.exports = function (error) {
flagError(error)

const viewObject = {
errorStack: error.stack,
pluginConfig: {
Expand Down

0 comments on commit e5cc521

Please sign in to comment.