Remove fs-extra #147
Remove fs-extra #147
Conversation
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/fs-extra@9.0.0 |
|
You can use Available since 16.7.0, marked as Experimental, though. I use it in scripts without trouble, but I’m not sure if I’d dare doing it in package code. |
| const util = require('util'); | ||
| const fs = require('graceful-fs'); | ||
| const {rimraf} = require('rimraf'); | ||
|
|
||
| const copyFile = util.promisify(fs.copyFile); |
There was a problem hiding this comment.
Btw, there’s a a promises API: https://nodejs.org/api/fs.html#promises-api
There was a problem hiding this comment.
Ah yes, I looked for it in my code but didn't see any usages, so I thought they must have appeared after v10.
There was a problem hiding this comment.
From the page above:
Version Changes v14.0.0 Exposed as require('fs/promises'). v11.14.0, v10.17.0 This API is no longer experimental. v10.1.0 The API is accessible via require('fs').promises only. v10.0.0 Added in: v10.0.0
So you can use it in 10+, but it'll be experimental, and getting at it might be annoying.
|
For now Maybe I should break this in a future version because for a soon-to-be feature I'll need a more recent version of |
|
You are correct, Node.js 10 and 12 are currently broken. Given that:
… I would say that it could be time to release that major version with some cleanups :) |
This comment was marked as outdated.
This comment was marked as outdated.
Wait, are these being copied into new elm-review packages? As in, they have to be run in the context of a new project? If so, I'd probably drop fs-extra there, because unless I'm misunderstanding this code, fs-extra isn't going to be there, right? Oh, or you could just exclude it from ts, or @ts-expect-error. Honestly, the last option might be best. It's not like an @ts-ignore, it asserts that it doesn't work, and fails once it starts working. |
| const promises = files.map((file) => | ||
| copyFile(path.join(srcFolder, file), path.join(destFolder, file)) | ||
| ); | ||
| return Promise.all(promises).then(() => undefined); |
There was a problem hiding this comment.
Why not this?
| return Promise.all(promises).then(() => undefined); | |
| return Promise.all(promises); |
Maybe I've forgotten something, but the noop shouldn't be necessary.
There was a problem hiding this comment.
I think it's to have the return type @returns {Promise<void>} instead of @returns {Promise<void[]>} or @returns {Promise<something[]>}
There was a problem hiding this comment.
I think it's to have the return type
@returns {Promise<void>}instead of@returns {Promise<void[]>}or@returns {Promise<something[]>}
Hmm. That makes sense. I would think that void[] is assignable to void, but maybe not.
Manual rebase of jfmengels#147. Co-authored-by: Jeroen Engels <jfm.engels@gmail.com>
|
Supersed by #303 |
Manual rebase of jfmengels#147. Co-authored-by: Jeroen Engels <jfm.engels@gmail.com>
Manual rebase of jfmengels#147. Co-authored-by: Jeroen Engels <jfm.engels@gmail.com>
Manual rebase of #147. Co-authored-by: Jeroen Engels <jfm.engels@gmail.com>
Fixes #120
Removes the dependency on
fs-extra. As @lydell pointed out,fs.copyFileSyncalready exists in corefs.fs.copy(which copies entire directories) however doesn't. I chose to fix this by hardcoding which files to copy (which is faster, though potentially error-prone 🤷)Now that
fs-extrais absent from the dependencies,tsccomplains about the files that reference it in thenew-package/folder. Those files are copied over and not included, but do usefs-extra(mostly because of the "need" to copy entire directories, ideally in a sync manner). I don't know how to suppress this error, and if I ignore the file, then ESLint complains that it can't parse it.If anyone has an idea on how to fix this, I'm all ears!