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

Update packages, configs + Node.js 20 #612

Merged
merged 30 commits into from
Jan 15, 2024
Merged

Update packages, configs + Node.js 20 #612

merged 30 commits into from
Jan 15, 2024

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Nov 20, 2023

Similar to #316 this PR brings all our dependencies up to date

Excluding two major bumps which I've split out:

  1. Upgrade to webpack v5 in Upgrade to webpack v5 #614
  2. Upgrade to WebdriverIO v8 in Upgrade to WebdriverIO v8 #615

Note: Updates to Preact have also been excluded since we'll need to update tests separately. For example, we currently require a mismatched Preact version via https://unpkg.com

Configs, tests and polyfills

We know the deprecated @wdio/sync package won't install so tests now use async/await instead

But we can remove more (see below) with config updates, especially now IE 8–10 support is clarified:

  1. Browserslist config updated with IE11+ to match
  2. Babel config presets updated for core-js@3 polyfills
  3. Babel config transforms for ES3 removed (prevented errors in IE8)
  4. Node.js version config .nvmrc updated to v20.9.0 lts/iron
  5. Husky setup moved to new .husky/pre-commit config format

Packages removed

The following packages were unnecessary or unused:

@wdio/sync
copy-webpack-plugin
dotenv
karma-chai
karma-chai-sinon
replace-bundle-webpack-plugin
sinon
sinon-chai

With more included in @babel/preset-env automatically now or to transform to ES3 for Internet Explorer 8:

@babel/plugin-proposal-class-properties
@babel/plugin-proposal-decorators
@babel/plugin-transform-member-expression-literals
@babel/plugin-transform-modules-commonjs
@babel/plugin-transform-property-literals

Package audit

See results from npm audit below

Before

found 181 vulnerabilities (3 low, 45 moderate, 104 high, 29 critical) in 2141 scanned packages
  run `npm audit fix` to fix 132 of them.
  43 vulnerabilities require semver-major dependency updates.
  6 vulnerabilities require manual review. See the full report for details.

After

11 vulnerabilities (3 moderate, 8 high)

To address all issues (including breaking changes), run:
  npm audit fix --force

See PRs #614 and #615 to reach 0 vulnerabilities

@colinrotherham colinrotherham changed the base branch from main to configure-node November 20, 2023 09:30
@rgarner
Copy link
Contributor

rgarner commented Nov 20, 2023

@colinrotherham Wow, thanks, this is a lot of work and looks great. The move to await (away from fibers?) has fixed my Promise issues with integration tests — npm test now completes with no warnings or errors. I had one issue on setup — npm install (from npm 6.14.18) downgraded all package.json integrity checksums from sha-512 to sha1, leaving me with a modified package-lock.json in my working tree. For anyone else upgrading with a similar issue (maybe this one?), here's what worked for me:

git checkout package-lock.json
rm -rf node_modules
npm cache clear --force # you may not need this, at your own risk, etc — I did need it, though
npm install

Now rebasing #611 to see how that goes (to add an integration test)...

EDIT: in that npm SHA issue above it mentions that it was an issue with 6.x. I wonder if it might be worth specifying an npm version (advisory only) in package.json? (ignorance: didn't know fixed npm version came packaged with node)

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Nov 20, 2023

Brilliant, thanks for confirming that

…I had one issue on setup — npm install (from npm 6.14.18) downgraded all package.json integrity checksums from sha-512 to sha1, leaving me with a modified package-lock.json in my working tree…

Sounds like we might be on different npm versions?

$ node -v
v14.21.3
$ npm -v
6.14.18

Hope this idea is alright, but I'll cherry your .nvmrc commit into this branch and we'll close PR #609

Gives us a single place to rebase from and I can work on raising the maximum Node.js version too

@rgarner
Copy link
Contributor

rgarner commented Nov 20, 2023

Sounds like we might be on different npm versions?

@colinrotherham we're both on 6.14.18 by the looks? Seems to be an issue with that particular npm major version that committers are saying "upgrade to 7" for (or rather, my reading is "this might happen in 6.x but we won't fix it").

Hope this idea is alright, but I'll cherry your .nvmrc commit into this branch and we'll close PR #609

Yep, good idea, #609 is looking pretty anaemic now.

@colinrotherham
Copy link
Contributor Author

colinrotherham commented Nov 20, 2023

Latest push highlights includes:

@colinrotherham colinrotherham force-pushed the package-updates branch 4 times, most recently from 3dd3500 to 927b0fa Compare November 20, 2023 20:19
@colinrotherham colinrotherham force-pushed the package-updates branch 2 times, most recently from 300418c to a119c39 Compare November 21, 2023 09:25
@colinrotherham colinrotherham changed the title Update all the packages Update all the packages + Node.js 20 Nov 21, 2023
@colinrotherham colinrotherham marked this pull request as ready for review November 21, 2023 10:13
@colinrotherham colinrotherham force-pushed the package-updates branch 3 times, most recently from 267ed78 to b335388 Compare November 21, 2023 15:34
@rgarner
Copy link
Contributor

rgarner commented Nov 21, 2023

@colinrotherham this is all working perfectly for me over on this branch, can run every type of test, so whatever I've wrought upon #611 is of my own rebase making and I shall unpick that in the morning!

package.json Show resolved Hide resolved
@colinrotherham colinrotherham changed the title Update all the packages + Node.js 20 Update packages, configs + Node.js 20 Nov 22, 2023
@colinrotherham colinrotherham self-assigned this Dec 4, 2023
E.g. Screenshots aren’t written on error without `./screenshots` directory
We don’t need to publish `preact` as a required dependency and it’s not bundled so we should use `peerDependencies` to specify which versions are supported instead

The README says “If you already use Preact in your application” with the example:

```
import preact from 'preact'
```
Migrates WebdriverIO tests away from the deprecated “sync” syntax which both unblocks Node.js 16+ upgrades in future and removes the requirement for a Python build step

See: #608
Removes the `copy-webpack-plugin` package since the dev server can proxy `accessible-autocomplete.min.css` from source instead
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to have everything a little more up to date, thanks for going through this (and having everything nicely split for review as well). ⛵

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants