-
Notifications
You must be signed in to change notification settings - Fork 794
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
chore: update dependencies #3539
Conversation
depenencies with a .x cannot be upgraded further
requested by Michael
Flakey test locally; checking CI
Requires “internet explorer”
deepEqual() assert failed because selenium now places a banner that throws off window height
deepEqual() assert failed because selenium now places a banner that throws off window height
…core into chore/update-dependencies
v11 introduces ESM
confirmed v10 sinon does not support IE11 for CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @WilcoFiers should also approve since I helped a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor points
| aria-attribute | axe-core support | | ||
| -------------- | ---------------- | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems odd. Should we turn off this part of the task? An empty table doesn't seem useful to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any other docs page that had an empty table, so this does stand out. Since it is generated dynamically, would disabling the rule have unexpected consequences in the future?
My thoughts now are to not disable the task, but instead alter aria-supported.js
to output an empty string if this might bite us later. If not, disabling the task or culling the code would certainly be easiest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something needs to be done there. It's definitely outdated. Perhaps we should change that page to ARIA 1.3. I'll open a ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll let you open a ticket and will mark this as resolved as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New issue for this is #3590.
"minami": "^1.2.3", | ||
"mocha": "^9.2.0", | ||
"node-notifier": "^8.0.1", | ||
"mocha": "9.x", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not Mocha 10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because version 10 drops IE 11 and Node 12 support.
https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#1000--2022-05-01
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spoke with @straker yesterday. Since we need the IE 11 support, we won't upgrade to Mocha 10 and will leave this comment as unresolved in the PR.
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
# Conflicts: # package-lock.json
6da694d
to
7a2c4d2
Compare
This PR is ready for security review, even though it has pending changes--those will be left open, see above. |
Resolved
Updates project dependencies to current versions when possible.
The following dependencies are one or more major versions behind and have not been upgraded to latest, but should be safe for minor and patch updates.
4.x
(latest is5.0.1
)Version
5
is breaking due to ESM only https://github.com/chalk/chalk/releases/tag/v5.0.05.x
(latest is6.1.0
)Version
6
is breaking due to ESM only https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c11.x
(latest is13.1.2
)Version
12
is breaking due to ESM only https://github.com/sindresorhus/globby/releases/tag/v12.0.01.x
(latest is2.3.3
)Version
2
is breaking due to ESM only https://github.com/mdevils/html-entities/releases/tag/v2.0.02.x
(latest is3.0.2
)Version
3
is breaking due to ESM only https://github.com/wooorm/markdown-table/releases/tag/3.0.09.x
(latest is10.0.0
)Version
10
drops IE 11 and Node 12 support https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#1000--2022-05-0111.x
(latest is14.0.0
)Version
12
is breaking due to ESM only https://github.com/sinonjs/sinon/blob/main/CHANGES.md#1200Version
10
is breaking due to IE 11 not supporting template literals https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#browser_compatibilitySeleniumn is pinned due to a bug, but should be easily updated in the future.
4.3.0
because4.3.1
introduces a breaking change from 4.3.0->4.3.1selenium-webdriver/chrome#setDefaultService()
https://github.com/SeleniumHQ/selenium/pull/10796/files#diff-6c87d95a2288e92e15a6bb17710c763c01c2290e679beb26220858f3218b6a62L260Also, it appears the selenium webdriver browser message "Chrome is being controlled by automated software" changes the window height when it appears between running the two axe runs. To fix
deepEqual
, we assign the height to be identical:axeRunPartialResult.testEnvironment = axeRunResult.testEnvironment;
ESBuild is upgraded to
0.14.7
(0.14.50 is latest
) due to flaky windows tests. This is the version that passed CI and was reasonably up-to-date.replaces: PR #3535