Remove fs-extra

[jfmengels]

Fixes #120

Removes the dependency on fs-extra. As @lydell pointed out, fs.copyFileSync already exists in core fs. 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 :shrug:)

Now that fs-extra is absent from the dependencies, tsc complains about the files that reference it in the new-package/ folder. Those files are copied over and not included, but do use fs-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!

[socket-security[bot]]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected]	filesystem Transitive: environment	+5	222 kB	ryanzim

🚮 Removed packages: npm/[email protected]

View full report↗︎

[lydell]

You can use fs.cpSync(src, dest, { recursive: true }) to copy whole dirs. https://nodejs.org/api/fs.html#fscpsyncsrc-dest-options

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.

[jfmengels]

For now elm-review supports v10 of Node.js and above, although I think that support for v10 and v12 is broken because of indirect dependencies :disappointed:, but supporting older versions including v14 is still the aim.

Maybe I should break this in a future version because for a soon-to-be feature I'll need a more recent version of glob which only supports ">=16 || 14 >=14.17". Maybe that would be a good time to cut a major version of the CLI :shrug:

[lydell]

You are correct, Node.js 10 and 12 are currently broken. Given that:

• Not very many people have complained about it.
• Node.js 10 and 12 have been EOL for quite some time now.
• You can release major versions of the CLI without causing problems like with the Elm package (which would make all rule packages incompatible).

… I would say that it could be time to release that major version with some cleanups :)

[lishaduck]

I think that support for v10 and v12 is broken because of indirect dependencies 😞, but supporting older versions including v14 is still the aim.

You are correct, Node.js 10 and 12 are currently broken.

EDIT: See #133

---

Thoughts I wrote down relating to majors, that none need read, but which people could, should they wish to, for some unknown reason:

• Node.js 10 and 12 have been EOL for quite some time now.
• You can release major versions of the CLI without causing problems like with the Elm package (which would make all rule packages incompatible).

If you're dropping node versions, drop to 18+, and just tell folks below that to use npm:elm-review@2, assuming elm releases are decoupled (so everything still works). After all, Node 18 is still maintained, but 16 uses an outdated OpenSSL, so it's out of support. Then again, I don't like losing the 2-2 though, as it helps me mentalize it (is it elm-pages v3 or v10? 😉). If you made a major with the elm package though, that might also be acceptable for people (i.e., breaking everywhere). On the other hand, that's majorly breaking to the ecosystem, but you could make a codemod, so maybe not terrible? Especially with @elm-review-bot? Oh, and this is a good read by the creator of SemVer himself: Major Version Numbers are Not Sacred.

[lishaduck]

Now that fs-extra is absent from the dependencies, tsc complains about the files that reference it in the new-package/ folder. Those files are copied over and not included, but do use fs-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.

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? Nope. I see, you're generating that dynamically. I'd make fs-extra a devdep, given that you can't inline the files b/c back compat. It is, after all, a devdep in the sense that it provides types.

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.