chore(deps): drop fs-extras and mkdirp dependencies #3230
Conversation
| @@ -109,6 +107,7 @@ | |||
| "eslint": "8.57.0", | |||
| "eslint-plugin-async-await": "0.0.0", | |||
| "eslint-plugin-import": "2.29.1", | |||
| "fs-extra": "11.2.0", | |||
There was a problem hiding this comment.
It's still used in tests, I didn't want to rewrite all of them. It's fine as a dev dependency
There was a problem hiding this comment.
that's fair, it seems to be used only in the test.chromium.js unit test, but it is not a big deal if we keep it as a dev dependency in the meantime. Let's keep that test as is 👍
| @@ -179,12 +178,13 @@ export class ChromiumExtensionRunner { | |||
|
|
|||
| if (userDataDir && profileDirName) { | |||
| // copy profile dir to this temp user data dir. | |||
| await fs.copy( | |||
| await fs.cp( | |||
There was a problem hiding this comment.
Added to Node 16.7 exactly 3 years ago
| @@ -273,7 +273,7 @@ export class ChromiumExtensionRunner { | |||
|
|
|||
| log.debug(`Creating reload-manager-extension in ${extPath}`); | |||
|
|
|||
| await asyncMkdirp(extPath); | |||
| await fs.mkdir(extPath, { recursive: true }); | |||
There was a problem hiding this comment.
Added to Node 10 (as promised)
fregante
changed the title
There was a problem hiding this comment.
@fregante thank you, I'm signing off this version of this PR as is (and merging it right after).
I also took a look to the mkdirp README and based what they are stating in it mkdir was basically already using the native recursive fs.mkdir feature available in nodejs > 10, besides in a few cases that we shouldn't be hitting and for a couple of cases wher the errors raised by the native implementation wasn't clear on why the call was failing (but the issues referenced seems to have been also fixed in th meantime, merged in nodejs v22 if I'm not mistaken, and so I'm not too worried about those to be honest).
I also agree that keeping fs-extra as a dev dependency in this PR is fine (so that we can keep test.chromium.js tests unchanged).
| @@ -109,6 +107,7 @@ | |||
| "eslint": "8.57.0", | |||
| "eslint-plugin-async-await": "0.0.0", | |||
| "eslint-plugin-import": "2.29.1", | |||
| "fs-extra": "11.2.0", | |||
There was a problem hiding this comment.
that's fair, it seems to be used only in the test.chromium.js unit test, but it is not a big deal if we keep it as a dev dependency in the meantime. Let's keep that test as is 👍
See review comments below