Skip to content
This repository has been archived by the owner on Mar 14, 2022. It is now read-only.

Commit

Permalink
Use fs.access instead of fs.open to avoid having to open and close fi…
Browse files Browse the repository at this point in the history
…le-descriptors.

NodeJS is deprecating the automatic closing of fileHandlers during garbage collection.

This commit updates the codebase to explicitly close the fileHandlers instead of relying on the deprecated behaviour.

Below is the message that is displayed when running obt prior to this commit:

"DeprecationWarning: Closing a FileHandle object on garbage collection is deprecated. Please close FileHandle objects explicitly using FileHandle.prototype.close(). In the future, an error will be thrown if a file descriptor is closed during garbage collection."
  • Loading branch information
JakeChampion committed Jul 14, 2021
1 parent 7894525 commit 3715752
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 34 deletions.
20 changes: 18 additions & 2 deletions lib/helpers/files.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
'use strict';

const { readFile, open } = require('fs/promises');
const { readFile, access } = require('fs/promises');
const { constants } = require('fs');
const path = require('path');
const denodeify = require('util').promisify;
const deglob = denodeify(require('deglob'));

const fileExists = (file) => open(file, 'r').then(() => true).catch(() => false);
/**
* Node.JS no longer has an fs.exists method.
* Instead we use the fs.access method and check we can read the file.
* fs.access will throw an error if the file does not exist.
* @param {string} file file-system path to the file you are wanting to check exists or not
* @returns {Promise.<boolean>} Whether the file exists
*/
async function fileExists(file) {
try {
await access(file, constants.R_OK);
return true;
} catch (error) {
return false;
}
}

function getBuildFolderPath(cwd) {
cwd = cwd || process.cwd();
Expand Down Expand Up @@ -165,6 +180,7 @@ function getSassIncludePaths (cwd, config = {sassIncludePaths: []}) {
].map(pathname => path.join(cwd, pathname)));
}

module.exports.fileExists = fileExists;
module.exports.readIfExists = readIfExists;
module.exports.getBuildFolderPath = getBuildFolderPath;
module.exports.getMainSassPath = getMainSassPath;
Expand Down
7 changes: 3 additions & 4 deletions lib/tasks/demo-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ const constructPolyfillUrl = require('../helpers/construct-polyfill-url');
const mustache = require('mustache');
const denodeify = require('util').promisify;

const fileExists = file => denodeify(fs.open)(file, 'r').then(() => true).catch(() => false);
const readFile = denodeify(fs.readFile);
const outputFile = denodeify(fs.outputFile);

function buildDemoSass(buildConfig) {
const src = path.join(buildConfig.cwd, '/' + buildConfig.demo.sass);
const dest = path.join(buildConfig.cwd, '/demos/local/');

return fileExists(src)
return files.fileExists(src)
.then(exists => {
if (!exists) {
const e = new Error('Sass file not found: ' + src);
Expand Down Expand Up @@ -46,7 +45,7 @@ function buildDemoJs(buildConfig) {
const src = path.join(buildConfig.cwd, '/' + buildConfig.demo.js);
const destFolder = path.join(buildConfig.cwd, '/demos/local/');
const dest = path.basename(buildConfig.demo.js);
return fileExists(src)
return files.fileExists(src)
.then(exists => {
if (!exists) {
const e = new Error('JavaScript file not found: ' + src);
Expand Down Expand Up @@ -110,7 +109,7 @@ function buildDemoHtml(buildConfig) {
return loadDemoData(buildConfig)
.then(demoData => {
data = demoData;
return fileExists(src);
return files.fileExists(src);
})
.then(exists => {
if (!exists) {
Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/test-sass-compilation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ const ListrRenderer = require('../helpers/listr-renderer');
const isCI = require('is-ci');
const { camelCase } = require('lodash');
const ftSass = require('@financial-times/sass');
const { readFile, open } = require('fs/promises');
const { readFile } = require('fs/promises');
const execa = require('execa');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);
async function compilationTest(cwd, { silent, brand } = {
silent: false,
brand: false
Expand Down Expand Up @@ -121,7 +120,7 @@ module.exports = function (cfg) {
});
},
skip: async () => {
const exists = await fileExists(await files.getMainSassPath(config.cwd));
const exists = await files.fileExists(await files.getMainSassPath(config.cwd));
return !exists;
}
};
Expand Down
4 changes: 1 addition & 3 deletions lib/tasks/verify-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ const path = require('path');
const ESLint = require('eslint').ESLint;
const denodeify = require('util').promisify;
const deglob = denodeify(require('deglob'));
const { open } = require('fs/promises');
const log = require('../helpers/log');
const {fileExists} = require('../helpers/files');
const isCI = require('is-ci');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);

async function lint (config) {
const hasJS = await projectHasJavaScriptFiles(config);

Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/verify-origami-json.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
'use strict';

const process = require('process');
const { readFile, open} = require('fs/promises');
const { readFile } = require('fs/promises');
const { fileExists} = require('../helpers/files');
const path = require('path');
const isCI = require('is-ci');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);

// https://origami.ft.com/docs/manifests/origami-json/#origamitype
// "component": A front-end component that follows the component specification
// "imageset": A set of images that have an alias on the Origami Image Service
Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/verify-package-json.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
'use strict';

const process = require('process');
const { readFile, open } = require('fs/promises');
const { readFile } = require('fs/promises');
const { fileExists} = require('../helpers/files');
const path = require('path');
const isCI = require('is-ci');
const semver = require('semver');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);

/**
* Checks whether description conforms to the origami package.json description specification.
* @param {any} description The value to check
Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/verify-readme.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
'use strict';

const process = require('process');
const { readFile, open } = require('fs/promises');
const { readFile } = require('fs/promises');
const path = require('path');
const isCI = require('is-ci');
const vfile = require('vfile');
const remark = require('remark');
const remarkLint = require('remark-lint');
const report = require('vfile-reporter');
const log = require('../helpers/log');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);
const { fileExists} = require('../helpers/files');

async function origamiJson(config) {
// Error if there is no readme to verify.
Expand Down
4 changes: 1 addition & 3 deletions lib/tasks/verify-sass.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ const process = require('process');
const path = require('path');
const denodeify = require('util').promisify;
const deglob = denodeify(require('deglob'));
const { open } = require('fs/promises');
const isCI = require('is-ci');
const execa = require('execa');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);
const { fileExists} = require('../helpers/files');
async function sassLint(config) {
const hasScss = await projectHasScssFiles(config);

Expand Down
18 changes: 11 additions & 7 deletions test/integration/helpers/fileExists.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
'use strict';

const open = require('fs/promises').open;
const { access } = require('fs/promises');
const { constants } = require('fs');

/**
* Node.JS no longer has an fs.exists method.
* Instead we use the fs.open method in read (`r`) mode.
* fs.open will throw an error if the file does not exist.
* Instead we use the fs.access method and check we can read the file.
* fs.access will throw an error if the file does not exist.
* @param {string} file file-system path to the file you are wanting to check exists or not
* @returns {Promise.<boolean>} Whether the file exists
*/
module.exports = function fileExists(file) {
return open(file, 'r')
.then(() => true)
.catch(() => false);
module.exports = async function fileExists(file) {
try {
await access(file, constants.R_OK);
return true;
} catch (error) {
return false;
}
};
5 changes: 2 additions & 3 deletions test/integration/verify/verify.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ const process = require('process');
const proclaim = require('proclaim');
const obtBinPath = require('../helpers/obtpath');
const rimraf = require('../helpers/delete');
const { writeFile, open } = require('fs/promises');
const fileExists = require('../helpers/fileExists');
const { writeFile } = require('fs/promises');
const tmpdir = require('../helpers/tmpdir');

const fileExists = file => open(file, 'r').then(() => true).catch(() => false);

describe('obt verify', function () {
let obt;

Expand Down

0 comments on commit 3715752

Please sign in to comment.