Bump dependencies (uuid, debug, dev-deps) #217
Conversation
chrisveness
commented
Nov 2, 2021
chrisveness
commented
- tests: fix ".end() was called twice" in contextstore.test.js
- use eslint:recommended rather than Egg config
- eslint-disable no-prototype-builtins for hasOwnProperty(CONTEXT_SESSION) (?)
- eslint-disable require-yield in cookie.test.js (?)
- replace defunct istanbul with c8
- tests: fix ".end() was called twice" in contextstore.test.js - use eslint:recommended rather than Egg config - eslint-disable no-prototype-builtins for hasOwnProperty(CONTEXT_SESSION) (?) - eslint-disable require-yield in cookie.test.js (?) - replace defunct istanbul with c8
|
npm WARN deprecated [email protected]: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. |
| }, | ||
| "engines": { | ||
| "node": ">=7.6" | ||
| "node": ">=10.0.0" |
There was a problem hiding this comment.
This makes this a breaking change, which'd mean a new major version - could be worth trying to split this PR up into a couple of smaller ones so non-breaking changes can be landed?
There was a problem hiding this comment.
From memory, I think it was dev-deps that prompted the engines.node bump, though I didn't make particular note of it.
I've never been quite sure about whether the engines.node should relate to app + dependencies, or app + dependencies + devDependencies.
I think the only substantive changes (ignoring dev-deps) are uuid & debug; [email protected] specifies engines.node as >=6.0, uuid doesn't mention engines.node.
I think I bumped engines.node to 10+ since eslint required that, but whether engines.node should depend on a devDependency, I'm not sure!
To further complicate the issue, [email protected] specifies engines.node as >=12: but having just run a test, the koa-session example.js runs perfectly well on [email protected].
If the koa-session maintainers feel it worthwhile, the deps could be bumped separately from the dev-deps, but perhaps engines.node should not depend on devDependencies, in which I was mistaken in bumping it up to 10.0.0 and it can validly stay on 7.6.0?
There was a problem hiding this comment.
I think it's ok to upgrade node to 10.0.0 since node<10 is already out of maintenance for a long time.
| @@ -394,12 +394,11 @@ describe('Koa Session External Context Store', () => { | |||
|
|
|||
| request(server) | |||
There was a problem hiding this comment.
if we remove done, we should return request to ensure this promise resolved.
There was a problem hiding this comment.
Is it correct like this? (I'm not familiar with this mode of usage of mocha).
| }, | ||
| "engines": { | ||
| "node": ">=7.6" | ||
| "node": ">=10.0.0" |
There was a problem hiding this comment.
I think it's ok to upgrade node to 10.0.0 since node<10 is already out of maintenance for a long time.
|
BUMP! Almost a year since I think we agreed this PR is ok? |
