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
Upgrade dependencies #440
Upgrade dependencies #440
Conversation
d56aac6
to
de9f3c4
Compare
Since rebasing and doing all the upgrades, Node 14 complains:
|
There's always one squeaky wheel, isn't there? :P |
And fix a couple of lint violations
In @sinonjs/eslint-config we have disabled all style rules and are using prettier's defaults, which are enforced by both commit hook and CI
3ec0de6
to
288fb86
Compare
Codecov ReportBase: 95.52% // Head: 95.52% // No change to project coverage
Additional details and impacted files@@ Coverage Diff @@
## main #440 +/- ##
=======================================
Coverage 95.52% 95.52%
=======================================
Files 2 2
Lines 648 648
=======================================
Hits 619 619
Misses 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
|
Fixed the Node 14 issue by adding |
Thank you |
@@ -50,6 +50,9 @@ | |||
"dependencies": { | |||
"@sinonjs/commons": "^2.0.0" | |||
}, | |||
"optionalDependencies": { |
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 means every (mac) consumer of this module will install this. Would be nice to figure out which dependency has messed up their dependency tree and fix that instead of hacking around it here
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.
@SimenB that's a fair point. Would you mind making a ticket for it, so we don't forget?
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.
AFAIK when looking at this, it was chokidar
. And this was only an issue with Node 14/NPM 6, not with any later version, so I was thinking it was more of an issue of it only being able to handle the v1 of the package-lock format, whereas the package-lock file has v2.
And I think we want all consumers of mac to install this? This package ensures it uses file system events instead of polling.
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.
AFAIK when looking at this, it was
chokidar
. And this was only an issue with Node 14/NPM 6, not with any later version, so I was thinking it was more of an issue of it only being able to handle the v1 of the package-lock format, whereas the package-lock file has v2.
Could we upgrade the version of npm used on CI so the lockfile is supported? Or stay on v1 I guess (unless newer npm just updates the format automatically)
And I think we want all consumers of mac to install this? This package ensures it uses file system events instead of polling.
@sinonjs/fake-timers
doesn't do any FS at all, tho, so it shouldn't come with fsevents
(or chokidar
).
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.
@@ -0,0 +1,5 @@ | |||
#!/usr/bin/env sh | |||
. "$(dirname -- "$0")/_/husky.sh" |
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.
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.
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.
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.
whaaat. What kind of NPM are you running? npm bin
works even if alias npm=pnpm
.
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 need to test this locally.
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.
Purpose (TL;DR) - mandatory