Update dependencies + fix breaking changes #2937

colinrotherham · 2022-10-21T16:22:09Z

The following package updates have been applied:

Breaking changes

Cheerio no longer encodes non-ASCII characters so .html() is now true to the HTML source. You'll see various test changes for ', ", £ etc replaced with ', ", £ as Nunjucks renders them

cheerio → ^1.0.0-rc.12

Major but compatible

No breaking changes

glob → ^8.0.3
jest-axe → ^6.0.0
puppeteer → ^19.1.0
yargs → ^17.6.0

Compatible

Minor and patch semver-compatible changes

@babel/core → ^7.19.6
@percy/cli → ^1.12.0
babel-jest → ^29.2.1
cookie-parser → ^1.4.6
eslint-plugin-jsdoc → ^39.3.14
eslint-plugin-promise → ^6.1.1
express → ^4.18.2
express-validator → ^6.14.2
govuk_frontend_toolkit → ^9.0.1
gulp-task-listing → ^1.1.1
jest → ^29.2.1
jest-environment-jsdom → ^29.2.1
map-stream → ^0.0.7
marked → ^4.1.1
nodemon → ^2.0.20
outdent → ^0.8.0
stylelint → ^14.14.0
undici → ^5.11.0
ws → ^8.9.0

Incompatible

Intentionally pinned back to older (not latest) minor/patch versions

html-validate → 7.5.0
rollup → 0.56.5

Intentionally ignored until we use ESM by default

del → ^7.0.0 – ESM export only, we need CommonJS
slash → ^5.0.0 – ESM export only, we need CommonJS

Intentionally ignored due to other major breaking changes

autoprefixer → ^10.4.12 – Breaks IE8 PostCSS plugins
cssnano → ^5.1.13 – Breaks IE8 PostCSS plugins
jquery → ^3.6.1 – Breaks IE8 support, required by legacy toolkits
gulp-better-rollup → ^4.0.1 – Incompatible rollup peerDependency

Added

Previously missing and only available as a child dependency

nunjucks → ^3.2.3

govuk-frontend-repository@ /path/to/project/govuk-frontend
└─┬ sassdoc@2.7.4
  └─┬ sassdoc-theme-default@2.8.3
    └── nunjucks@3.2.2

Removed

These packages were either unused or not needed

@percy/logger
expect-puppeteer
gulp-eol
fsevents

36degrees

Can we capture a bit more info (or the failing test output?) in the commit message for 9ab8926? I didn't really understand what the problem was until I tried to install 7.6.0 locally.

colinrotherham · 2022-10-21T16:42:12Z

lib/file-helper.js

@@ -100,7 +100,7 @@ const getComponentData = async (componentName) => {
 /**
 * Load all components' data
 *
- * @returns {Promise<ComponentData>[]} Components' data
+ * @returns {Promise<ComponentData[]>} Components' data


It's not an array of promises, it's a promise that resolves to an array

colinrotherham · 2022-10-21T16:42:30Z

lib/jest-helpers.js

@@ -73,7 +73,7 @@ function callMacro (macroName, macroPath, params = [], callBlock = false) {
 * @param {object} options - options to pass to the component macro
 * @param {string} [callBlock] - if provided, the macro is called using the
 *   Nunjucks call tag, with the callBlock passed as the contents of the block
- * @returns {import('cheerio')} a jQuery-like representation of the macro output
+ * @returns {import('cheerio').CheerioAPI} a jQuery-like representation of the macro output


They've shuffled their types around in the latest update

colinrotherham · 2022-10-21T16:45:52Z

src/govuk/components/error-summary/error-summary.yaml

@@ -120,7 +120,7 @@ examples:
  hidden: true
  data:
    titleText: There is a problem
-    descriptionText: See errors below (▼)
+    descriptionText: See errors below (>)


I've changed this character ▼ → > as it wasn't actually something that Nunjucks would escape. We'll now see > → > which asserts that HTML is escaped

colinrotherham · 2022-10-21T16:48:39Z

src/govuk/components/input/template.test.js

@@ -310,23 +310,23 @@ describe('Input', () => {

      const $prefix = $('.govuk-form-group > .govuk-input__wrapper > .govuk-input__prefix')

-      expect($prefix.html()).toEqual('&#xA3;')
+      expect($prefix.html()).toEqual('£')


We definitely didn't have £ as £ in the HTML

(Confirmation that it was cheerio@1.0.0-rc.3 encoding things it shouldn't)

Includes `nunjucks` as strangely we never installed it

Ensures that `.html()` now contains the original HTML rather than incorrectly-encoded safe text that differed to the source Fix made in https://github.com/cheeriojs/cheerio/releases/tag/v1.0.0-rc.4 “We will no longer encode non-ASCII characters by default.”

Avoids issue where Cookie Banner “full banner hidden” example fails HTML validation for: 1. Rule: empty-heading https://html-validate.org/rules/empty-heading.html “<h2> cannot be empty, must have text content” 2. Rule: wcag/h30 https://html-validate.org/rules/wcag/h30.html “Anchor link must have a text describing its purpose” Both issues appear to be because `html-validate@7.6.0` thinks we’ve hidden the heading and link text in a non-accessible way: https://gitlab.com/html-validate/html-validate/-/commit/41ba7f9c4bb25bc543d66010331a0577d4c8f534

Due to package updates `rollup@0.56.5` can now be hoisted

We’re not using any of the Jest assertions

Primarily used to add new lines to HTML in #106 but isn’t needed anymore

It’s already flagged as an `optionalDependency` in the packages that use it: https://github.com/paulmillr/chokidar/blob/3.5.3/package.json#L25 https://github.com/facebook/jest/blob/v29.2.1/packages/jest-haste-map/package.json#L39

See: https://github.com/puppeteer/puppeteer/releases/tag/puppeteer-core-v19.0.0

colinrotherham · 2022-10-24T13:00:03Z

Can we capture a bit more info (or the failing test output?) in the commit message for 9ab8926? I didn't really understand what the problem was until I tried to install 7.6.0 locally.

Thanks @36degrees, mind having a look now? 6d144b0

colinrotherham · 2022-10-24T13:17:12Z

We had another dependency update to eslint-plugin-jsdoc:
https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v39.3.24

It now understands @returns {undefined | boolean} union types etc

This means we no longer need JSDoc @returns code changes

domoscargin · 2022-10-24T13:57:58Z

Ignore me - was comparing the wrong things

36degrees · 2022-10-24T13:58:03Z

Are the last two commits (1ed6172, aa59153) what you intended?

They have very similar commit messages and your last comment suggests you might have intended to remove the changes in 1ed6172.

domoscargin · 2022-10-24T10:56:49Z

package.json

@@ -60,7 +60,6 @@
    "govuk-elements-sass": "3.1.3",
    "gulp": "^4.0.2",
    "gulp-better-rollup": "3.1.0",
-    "gulp-eol": "^0.2.0",


As far as I understand, the changes from #106 for HTML files no longer happen, and the existing use of this just adds a line to some JS files (like dist/assets/govuk-frontend-4.3.1.min.js, for example), correct? In which case, our .gitattributes settings make this redundant, hence the removal?

Was primarily whether we're still worried if a final line ending is missing or not?

I didn't think it was worth keeping an old Gulp package around

domoscargin · 2022-10-24T14:17:37Z

If I run npm-check-updates, there's some compatible changes:

 body-parser           ^1.18.3  →   ^1.20.1
 eslint                ^8.25.0  →   ^8.26.0
 eslint-plugin-jsdoc  ^39.3.14  →  ^39.3.24

Are there reasons not to include these?

Latest ESLint plugin understands `@returns {undefined | false}` union types etc

colinrotherham · 2022-10-24T14:44:01Z

@domoscargin Yeah we have those ones already

npm ls body-parser
npm ls eslint
npm ls eslint-plugin-jsdoc

They got picked up by npm update so the semver range hasn't updated

Shall I update them if it helps explain that an update has gone in?

colinrotherham · 2022-10-24T14:49:29Z

Are the last two commits (1ed6172, aa59153) what you intended?

They have very similar commit messages and your last comment suggests you might have intended to remove the changes in 1ed6172.

@36degrees Ah thanks no. I've dropped the commit—it wasn't needed after all.

There was a momentary panic + git dance after yesterday's eslint-plugin-jsdoc@39.3.23 breaking changes were then fixed by eslint-plugin-jsdoc@39.3.24. Landed just as I'd changed some code 🙈

Conditional undefined returns are detected once more via gajus/eslint-plugin-jsdoc#925

domoscargin · 2022-10-24T14:49:35Z

@domoscargin Yeah we have those ones already
npm ls body-parser
npm ls eslint
npm ls eslint-plugin-jsdoc
They got picked up by npm update so the semver range hasn't updated

Shall I update them if it helps explain that an update has gone in?

Might be neatness for neatness sake, but I wouldn't mind them being updated.

colinrotherham · 2022-10-24T15:22:05Z

@domoscargin All sorted 👍

Running npm outdated now gives:

Package             Current  Wanted   Latest  Location                         Depended by
autoprefixer          9.8.8   9.8.8  10.4.12  node_modules/autoprefixer        govuk-frontend
cssnano              4.1.11  4.1.11   5.1.13  node_modules/cssnano             govuk-frontend
del                   6.1.1   6.1.1    7.0.0  node_modules/del                 govuk-frontend
gulp-better-rollup    3.1.0   3.1.0    4.0.1  node_modules/gulp-better-rollup  govuk-frontend
html-validate         7.5.0   7.5.0    7.7.0  node_modules/html-validate       govuk-frontend
jquery               1.12.4  1.12.4    3.6.1  node_modules/jquery              govuk-frontend
rollup               0.56.5  0.56.5    3.2.3  node_modules/rollup              govuk-frontend
slash                 3.0.0   3.0.0    5.0.0  node_modules/slash               govuk-frontend

I've updated the PR description to explain the other ignored major packages: #2937 (comment)

Most interesting are del and slash which we could use if we default task scripts to ESM

36degrees

Can we capture a bit more info (or the failing test output?) in the commit message for 9ab8926? I didn't really understand what the problem was until I tried to install 7.6.0 locally.

Thanks @36degrees, mind having a look now? 6d144b0

Thanks, that's great 👍🏻 Changes all look good to me, but I would like @domoscargin to approve as well before this is merged, given he's also flagged a few things.

domoscargin

Everything looks good for me locally on this. No obvious diff oddities, the scripts seem to run fine.

Nice!

colinrotherham · 2022-10-24T16:42:50Z

Fab, thanks @domoscargin + @36degrees

Wonder if configuring dependabot.yaml allow: all would help keep us up to date?

Rather than just the default security updates?

colinrotherham requested a review from a team as a code owner October 21, 2022 16:22

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 21, 2022 16:22 Inactive

colinrotherham added dependencies Pull requests that update a dependency file Frontend squad labels Oct 21, 2022

colinrotherham force-pushed the package-updates branch from 1dbfafb to 6a5af3d Compare October 21, 2022 16:33

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 21, 2022 16:33 Inactive

36degrees reviewed Oct 21, 2022

View reviewed changes

colinrotherham commented Oct 21, 2022

View reviewed changes

colinrotherham force-pushed the package-updates branch from 6a5af3d to 7865735 Compare October 21, 2022 16:42

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 21, 2022 16:43 Inactive

colinrotherham commented Oct 21, 2022

View reviewed changes

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 21, 2022 16:56 Inactive

Base automatically changed from postcss-config-remove-oldie to main October 21, 2022 16:56

colinrotherham force-pushed the package-updates branch from 18a86e9 to d1c486b Compare October 21, 2022 17:15

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 21, 2022 17:16 Inactive

domoscargin self-requested a review October 24, 2022 10:45

colinrotherham added 10 commits October 24, 2022 13:48

Package updates

c41889f

Includes `nunjucks` as strangely we never installed it

Fix getComponentsData() @returns type

a4ed4f3

Pin to rollup@0.56.5 for legacy IE8 support

011c731

Due to package updates `rollup@0.56.5` can now be hoisted

Remove @percy/logger (child dependency)

58f6a03

Remove expect-puppeteer (not needed)

7dc7ae9

We’re not using any of the Jest assertions

Remove gulp-eol (not needed)

877bd41

Primarily used to add new lines to HTML in #106 but isn’t needed anymore

Remove optional fsevents (not needed)

95f7efa

It’s already flagged as an `optionalDependency` in the packages that use it: https://github.com/paulmillr/chokidar/blob/3.5.3/package.json#L25 https://github.com/facebook/jest/blob/v29.2.1/packages/jest-haste-map/package.json#L39

Ensure new Puppeteer download location is cached

8ad0084

See: https://github.com/puppeteer/puppeteer/releases/tag/puppeteer-core-v19.0.0

colinrotherham force-pushed the package-updates branch from d1c486b to 8ad0084 Compare October 24, 2022 12:58

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 24, 2022 12:58 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 24, 2022 13:13 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 24, 2022 13:50 Inactive

domoscargin reviewed Oct 24, 2022

View reviewed changes

Fix JSDoc require-returns-check rule

57edd77

Latest ESLint plugin understands `@returns {undefined | false}` union types etc

colinrotherham force-pushed the package-updates branch from aa59153 to 57edd77 Compare October 24, 2022 14:43

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2937 October 24, 2022 14:43 Inactive

36degrees approved these changes Oct 24, 2022

View reviewed changes

domoscargin approved these changes Oct 24, 2022

View reviewed changes

colinrotherham merged commit df56a74 into main Oct 24, 2022

colinrotherham deleted the package-updates branch October 24, 2022 16:43

colinrotherham mentioned this pull request Oct 24, 2022

Ensure tests fail for browser exceptions #2933

Merged

This was referenced Nov 1, 2022

Update all dependencies using Dependabot #2950

Merged

Bump nth-check and cssnano #2953

Closed

Update all dependencies using Dependabot #2960

Closed

This was referenced Nov 8, 2022

Migrate Gulp and Node.js build tasks to ESM #2982

Merged

Enable package.json updates for Dependabot #2992

Merged

colinrotherham mentioned this pull request Nov 15, 2022

Upgrade to postcss version 8 is blocked on oldie #2469

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies + fix breaking changes #2937

Update dependencies + fix breaking changes #2937

colinrotherham commented Oct 21, 2022 •

edited

Loading

36degrees left a comment

colinrotherham Oct 21, 2022

colinrotherham Oct 21, 2022

colinrotherham Oct 21, 2022

colinrotherham Oct 21, 2022

colinrotherham commented Oct 24, 2022 •

edited

Loading

colinrotherham commented Oct 24, 2022 •

edited

Loading

domoscargin commented Oct 24, 2022 •

edited

Loading

36degrees commented Oct 24, 2022

domoscargin Oct 24, 2022 •

edited

Loading

colinrotherham Oct 24, 2022

domoscargin Oct 24, 2022

domoscargin commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

domoscargin commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

36degrees left a comment

domoscargin left a comment

colinrotherham commented Oct 24, 2022

Update dependencies + fix breaking changes #2937

Update dependencies + fix breaking changes #2937

Conversation

colinrotherham commented Oct 21, 2022 • edited Loading

Breaking changes

Major but compatible

Compatible

Incompatible

Added

Removed

36degrees left a comment

Choose a reason for hiding this comment

colinrotherham Oct 21, 2022

Choose a reason for hiding this comment

colinrotherham Oct 21, 2022

Choose a reason for hiding this comment

colinrotherham Oct 21, 2022

Choose a reason for hiding this comment

colinrotherham Oct 21, 2022

Choose a reason for hiding this comment

colinrotherham commented Oct 24, 2022 • edited Loading

colinrotherham commented Oct 24, 2022 • edited Loading

domoscargin commented Oct 24, 2022 • edited Loading

36degrees commented Oct 24, 2022

domoscargin Oct 24, 2022 • edited Loading

Choose a reason for hiding this comment

colinrotherham Oct 24, 2022

Choose a reason for hiding this comment

domoscargin Oct 24, 2022

Choose a reason for hiding this comment

domoscargin commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

domoscargin commented Oct 24, 2022

colinrotherham commented Oct 24, 2022

36degrees left a comment

Choose a reason for hiding this comment

domoscargin left a comment

Choose a reason for hiding this comment

colinrotherham commented Oct 24, 2022

colinrotherham commented Oct 21, 2022 •

edited

Loading

colinrotherham commented Oct 24, 2022 •

edited

Loading

colinrotherham commented Oct 24, 2022 •

edited

Loading

domoscargin commented Oct 24, 2022 •

edited

Loading

domoscargin Oct 24, 2022 •

edited

Loading