-
Notifications
You must be signed in to change notification settings - Fork 200
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(deps): bump rimraf from 2.6.2 to 5.0.10 #358
base: main
Are you sure you want to change the base?
Conversation
src/diff-snapshot.js
Outdated
@@ -271,10 +271,10 @@ function diffImageToSnapshot(options) { | |||
result = { added: true }; | |||
} else { | |||
const receivedSnapshotPath = path.join(receivedDir, `${snapshotIdentifier}${receivedPostfix}.png`); | |||
rimraf.sync(receivedSnapshotPath); | |||
rimrafSync(receivedSnapshotPath, { glob: true }); |
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.
Is glob: true
required 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.
The test cases pass without glob: true
here.
Without it in integration.spec.js
an error is thrown stating "Illegal characters in path.".
Since the path came from options then I´m not sure if it can become a problem without setting glob to true so I added it here as well. If you think this assumption is wrong then I have no problem removing it.
src/diff-snapshot.js
Outdated
@@ -18,7 +18,7 @@ const path = require('path'); | |||
const pixelmatch = require('pixelmatch'); | |||
const ssim = require('ssim.js'); | |||
const { PNG } = require('pngjs'); | |||
const rimraf = require('rimraf'); | |||
const { rimrafSync } = require('rimraf'); |
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.
It would be better to remove the usage of rimraf
from this file. See https://github.com/es-tooling/module-replacements/blob/main/docs/modules/rimraf.md for tips on how to do that
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.
Thanks for the suggestion, updated the code.
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.
thanks!
@Matthew-Mallimo do you think you could take another look at this? It would fix a number of deprecation warnings that are currently being printed by the library
Happy Thanksgiving!
Description
Fixes #355
Motivation and Context
How Has This Been Tested?
Types of Changes
Checklist:
What is the Impact to Developers Using Jest-Image-Snapshot?