Skip to content
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.

Sign up for GitHub

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

refactor: replace fs-extra with newer fs built-ins #5284

Merged
merged 2 commits into from
Jan 2, 2025
Merged

refactor: replace fs-extra with newer fs built-ins #5284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f4c344
Replace fs-extra with newer fs built-ins
ericcornelissen Dec 6, 2024
6fd91c4
Drop 14 and 16 in mocha.yml
JoshuaKGoldberg Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/mocha.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:
with:
os: 'ubuntu-latest,windows-latest'
# The 22.11.0 is instead of 22 per https://github.com/mochajs/mocha/issues/5278
node-versions: '14,16,18,20,22.11.0'
node-versions: '18,20,22.11.0'
npm-script: test-node:${{ matrix.test-part }}
coverage: ${{ matrix.coverage }}

Expand Down
26 changes: 0 additions & 26 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@
"cross-env": "^7.0.2",
"eslint": "^8.56.0",
"fail-on-errors-webpack-plugin": "^3.0.0",
"fs-extra": "^10.0.0",
"globals": "^13.24.0",
"installed-check": "^9.3.0",
"jsdoc": "^3.6.7",
Expand Down
13 changes: 7 additions & 6 deletions test/integration/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

const escapeRegExp = require('escape-string-regexp');
const os = require('os');
const fs = require('fs-extra');
const fs = require('fs');
const fsP = require('fs/promises');
const {format} = require('util');
const path = require('path');
const Base = require('../../lib/reporters/base');
Expand Down Expand Up @@ -487,7 +488,7 @@ const touchRef = new Date();
* @param {string} filepath - Path to file
*/
function touchFile(filepath) {
fs.ensureDirSync(path.dirname(filepath));
fs.mkdirSync(path.dirname(filepath), { recursive: true });
try {
fs.utimesSync(filepath, touchRef, touchRef);
} catch (e) {
Expand Down Expand Up @@ -519,21 +520,21 @@ function replaceFileContents(filepath, pattern, replacement) {
*/
function copyFixture(fixtureName, dest) {
const fixtureSource = resolveFixturePath(fixtureName);
fs.ensureDirSync(path.dirname(dest));
fs.copySync(fixtureSource, dest);
fs.mkdirSync(path.dirname(dest), { recursive: true });
fs.cpSync(fixtureSource, dest);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the CI logs, it seems this API is (the only one) missing from Node.js v14. It looks like that version of Node.js only has functions to copy individual files.

@JoshuaKGoldberg I have three suggestions for moving forward:

  1. Write a custom implementation of fs.cpSync here.
  2. Postpone this change until Node.js v14 is no longer run in CI.
  3. Drop the Node.js v14 job since it's no longer supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun timing: #5279 just dropped v14 in CI. If you update this branch from main it should be good to go! 🚀

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it is updated from main. Investigating...

}

/**
* Creates a temporary directory
* @returns {Promise<CreateTempDirResult>} Temp dir path and cleanup function
*/
const createTempDir = async () => {
const dirpath = await fs.mkdtemp(path.join(os.tmpdir(), 'mocha-'));
const dirpath = await fsP.mkdtemp(path.join(os.tmpdir(), 'mocha-'));
return {
dirpath,
removeTempDir: async () => {
if (!process.env.MOCHA_TEST_KEEP_TEMP_DIRS) {
return fs.remove(dirpath);
return fs.rmSync(dirpath, { recursive: true, force: true });
}
}
};
Expand Down
4 changes: 2 additions & 2 deletions test/integration/options/watch.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

const fs = require('fs-extra');
const fs = require('fs');
const path = require('path');
const {
copyFixture,
Expand Down Expand Up @@ -131,7 +131,7 @@ describe('--watch', function () {
[testFile, '--watch-files', 'lib/**/*.xyz'],
tempDir,
() => {
fs.removeSync(watchedFile);
fs.rmSync(watchedFile, { recursive: true, force: true });
}
).then(results => {
expect(results, 'to have length', 2);
Expand Down
Loading