Skip to content

Commit

Permalink
chore: remove unnecessary abstractions of filesystem functions (#5320)
Browse files Browse the repository at this point in the history
* chore: remove unnecessary abstractions of filesystem functions

* chore: update contributors field

Co-authored-by: danez <danez@users.noreply.github.com>
  • Loading branch information
danez and danez authored Dec 19, 2022
1 parent 6102e77 commit d3dfa17
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 87 deletions.
1 change: 0 additions & 1 deletion npm-shrinkwrap.json

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
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@
"cron-parser": "^4.2.1",
"debug": "^4.1.1",
"decache": "^4.6.0",
"del": "^6.0.0",
"dot-prop": "^6.0.0",
"dotenv": "^16.0.0",
"env-paths": "^2.2.0",
Expand Down
34 changes: 1 addition & 33 deletions src/lib/fs.cjs
Original file line number Diff line number Diff line change
@@ -1,26 +1,8 @@
// @ts-check
const {
constants,
promises: { access, readFile, rm, stat },
promises: { access, stat },
} = require('fs')
const { version } = require('process')

const del = require('del')
const { gte, parse } = require('semver')

const NODE_VERSION = parse(version)

/**
* reads a file async and catches potential errors
* @param {string} filePath
*/
const readFileAsyncCatchError = async (filePath) => {
try {
return { content: await readFile(filePath, 'utf-8') }
} catch (error) {
return { error }
}
}

const fileExistsAsync = async (filePath) => {
try {
Expand All @@ -31,18 +13,6 @@ const fileExistsAsync = async (filePath) => {
}
}

/**
* Removes a directory recursively and async
* @param {string} path
* @returns {Promise<void>}
*/
const rmdirRecursiveAsync = async (path) => {
if (gte(NODE_VERSION, '14.14.0')) {
return await rm(path, { force: true, recursive: true })
}
await del(path, { force: true })
}

/**
* calls stat async with a function and catches potential errors
* @param {string} filePath
Expand Down Expand Up @@ -78,6 +48,4 @@ module.exports = {
fileExistsAsync,
isDirectoryAsync,
isFileAsync,
readFileAsyncCatchError,
rmdirRecursiveAsync,
}
5 changes: 3 additions & 2 deletions src/utils/deploy/deploy-site.mjs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { rm } from 'fs/promises'

import cleanDeep from 'clean-deep'
import tempy from 'tempy'

import { deployFileNormalizer, getDistPathIfExists, isEdgeFunctionFile } from '../../lib/edge-functions/deploy.mjs'
import { rmdirRecursiveAsync } from '../../lib/fs.cjs'
import { warn } from '../command-helpers.mjs'

import {
Expand Down Expand Up @@ -170,7 +171,7 @@ For more information, visit https://ntl.fyi/cli-native-modules.`)
phase: 'stop',
})

await rmdirRecursiveAsync(tmpDir)
await rm(tmpDir, { force: true, recursive: true })

const deployManifest = {
deployId,
Expand Down
9 changes: 4 additions & 5 deletions src/utils/detect-server-settings.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// @ts-check
import { readFile } from 'fs/promises'
import { EOL } from 'os'
import path from 'path'
import process from 'process'
Expand All @@ -8,8 +9,6 @@ import fuzzy from 'fuzzy'
import getPort from 'get-port'
import isPlainObject from 'is-plain-obj'

import { readFileAsyncCatchError } from '../lib/fs.cjs'

import { NETLIFYDEVWARN, chalk, log } from './command-helpers.mjs'
import { acquirePort } from './dev.mjs'
import { getInternalFunctionsDir } from './functions/index.mjs'
Expand All @@ -34,9 +33,9 @@ const readHttpsSettings = async (options) => {
throw new TypeError(`Certificate file configuration should be a string`)
}

const [{ content: key, error: keyError }, { content: cert, error: certError }] = await Promise.all([
readFileAsyncCatchError(keyFile),
readFileAsyncCatchError(certFile),
const [{ reason: keyError, value: key }, { reason: certError, value: cert }] = await Promise.allSettled([
readFile(keyFile, 'utf-8'),
readFile(certFile, 'utf-8'),
])

if (keyError) {
Expand Down
8 changes: 4 additions & 4 deletions src/utils/lm/install.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-check
import { appendFile, copyFile, readFile, writeFile } from 'fs/promises'
import { appendFile, copyFile, readFile, rm, writeFile } from 'fs/promises'
import os from 'os'
import path, { dirname } from 'path'
import process from 'process'
Expand All @@ -11,7 +11,7 @@ import Listr from 'listr'
import pathKey from 'path-key'

import { fetchLatestVersion, shouldFetchLatestVersion } from '../../lib/exec-fetcher.mjs'
import { fileExistsAsync, rmdirRecursiveAsync } from '../../lib/fs.cjs'
import { fileExistsAsync } from '../../lib/fs.cjs'
import { normalizeBackslash } from '../../lib/path.mjs'
import { getLegacyPathInHome, getPathInHome } from '../../lib/settings.cjs'
import { chalk } from '../command-helpers.mjs'
Expand Down Expand Up @@ -92,7 +92,7 @@ const installedWithPackageManager = async function () {

const installHelper = async function () {
// remove any old versions that might still exist in `~/.netlify/helper/bin`
await rmdirRecursiveAsync(getLegacyBinPath())
await rm(getLegacyBinPath(), { force: true, recursive: true })
const binPath = getBinPath()
const shouldFetch = await shouldFetchLatestVersion({
binPath,
Expand Down Expand Up @@ -277,7 +277,7 @@ const cleanupShell = async function () {

export const uninstall = async function () {
await Promise.all([
rmdirRecursiveAsync(getHelperPath()),
rm(getHelperPath(), { force: true, recursive: true }),
removeConfig(GIT_CONFIG, getGitConfigContent(getGitConfigPath())),
cleanupShell(),
])
Expand Down
13 changes: 4 additions & 9 deletions tests/integration/640.command.recipes.test.cjs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
const { readFile } = require('fs/promises')
const { resolve } = require('path')

const test = require('ava')
const execa = require('execa')

const { readFileAsyncCatchError } = require('../../src/lib/fs.cjs')

const callCli = require('./utils/call-cli.cjs')
const cliPath = require('./utils/cli-path.cjs')
const { CONFIRM, answerWithValue, handleQuestions } = require('./utils/handle-questions.cjs')
Expand Down Expand Up @@ -35,10 +34,8 @@ test('Generates a new VS Code settings file if one does not exist', async (t) =>

await childProcess

const { content, error } = await readFileAsyncCatchError(`${builder.directory}/.vscode/settings.json`)
const settings = JSON.parse(content)
const settings = JSON.parse(await readFile(`${builder.directory}/.vscode/settings.json`))

t.is(error, undefined)
t.is(settings['deno.enable'], true)
t.is(settings['deno.importMap'], '.netlify/edge-functions-import-map.json')
t.deepEqual(settings['deno.enablePaths'], ['netlify/edge-functions'])
Expand Down Expand Up @@ -68,10 +65,8 @@ test('Updates an existing VS Code settings file', async (t) => {

await childProcess

const { content, error } = await readFileAsyncCatchError(`${builder.directory}/.vscode/settings.json`)
const settings = JSON.parse(content)
const settings = JSON.parse(await readFile(`${builder.directory}/.vscode/settings.json`))

t.is(error, undefined)
t.is(settings.someSetting, 'value')
t.is(settings['deno.enable'], true)
t.is(settings['deno.importMap'], '.netlify/edge-functions-import-map.json')
Expand All @@ -97,7 +92,7 @@ test('Does not generate a new VS Code settings file if the user does not confirm

await childProcess

const { error } = await readFileAsyncCatchError(`${builder.directory}/.vscode/settings.json`)
const error = await t.throwsAsync(() => readFile(`${builder.directory}/.vscode/settings.json`))
t.is(error.code, 'ENOENT')
})
})
6 changes: 2 additions & 4 deletions tests/integration/utils/mock-execa.cjs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
const { writeFile } = require('fs').promises
const { rm, writeFile } = require('fs').promises
const { pathToFileURL } = require('url')

const tempy = require('tempy')

const { rmdirRecursiveAsync } = require('../../../src/lib/fs.cjs')

// Saves to disk a JavaScript file with the contents provided and returns
// an environment variable that replaces the `execa` module implementation.
// A cleanup method is also returned, allowing the consumer to remove the
Expand All @@ -20,7 +18,7 @@ const createMock = async (contents) => {
}
const cleanup = () =>
// eslint-disable-next-line promise/prefer-await-to-then
rmdirRecursiveAsync(path).catch(() => {
rm(path, { force: true, recursive: true }).catch(() => {
// no-op
})

Expand Down
6 changes: 2 additions & 4 deletions tests/integration/utils/site-builder.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-check
const { copyFile, mkdir, unlink, writeFile } = require('fs').promises
const { copyFile, mkdir, rm, unlink, writeFile } = require('fs').promises
const os = require('os')
const path = require('path')
const process = require('process')
Expand All @@ -10,8 +10,6 @@ const tempDirectory = require('temp-dir')
const { toToml } = require('tomlify-j0.4')
const { v4: uuidv4 } = require('uuid')

const { rmdirRecursiveAsync } = require('../../../src/lib/fs.cjs')

const ensureDir = (file) => mkdir(file, { recursive: true })

const createSiteBuilder = ({ siteName }) => {
Expand Down Expand Up @@ -184,7 +182,7 @@ const createSiteBuilder = ({ siteName }) => {
return builder
},
cleanupAsync: async () => {
await rmdirRecursiveAsync(directory).catch((error) => {
await rm(directory, { force: true, recursive: true }).catch((error) => {
console.warn(error)
})
return builder
Expand Down
13 changes: 6 additions & 7 deletions tests/unit/utils/get-global-config.test.mjs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { copyFile, mkdir, readFile, unlink, writeFile } from 'fs/promises'
import { copyFile, mkdir, readFile, rm, unlink, writeFile } from 'fs/promises'
import os from 'os'
import { join } from 'path'

import { afterAll, beforeAll, beforeEach, expect, test } from 'vitest'

import { rmdirRecursiveAsync } from '../../../src/lib/fs.cjs'
import { getLegacyPathInHome, getPathInHome } from '../../../src/lib/settings.cjs'
import getGlobalConfig, { resetConfigCache } from '../../../src/utils/get-global-config.mjs'

Expand All @@ -28,13 +27,13 @@ afterAll(async () => {
await unlink(tmpConfigBackupPath)
} catch {}
// Remove legacy config path
await rmdirRecursiveAsync(getLegacyPathInHome([]))
await rm(getLegacyPathInHome([]), { force: true, recursive: true })
})

beforeEach(async () => {
// Remove config dirs
await rmdirRecursiveAsync(getPathInHome([]))
await rmdirRecursiveAsync(getLegacyPathInHome([]))
await rm(getPathInHome([]), { force: true, recursive: true })
await rm(getLegacyPathInHome([]), { force: true, recursive: true })
// Make config dirs
await mkdir(getPathInHome([]))
await mkdir(getLegacyPathInHome([]))
Expand Down Expand Up @@ -63,8 +62,8 @@ test('should not throw if legacy config is invalid JSON', async () => {

test("should create config in netlify's config dir if none exists and store new values", async () => {
// Remove config dirs
await rmdirRecursiveAsync(getPathInHome([]))
await rmdirRecursiveAsync(getLegacyPathInHome([]))
await rm(getPathInHome([]), { force: true, recursive: true })
await rm(getLegacyPathInHome([]), { force: true, recursive: true })
const globalConfig = await getGlobalConfig()
globalConfig.set('newProp', 'newValue')
const configFile = JSON.parse(await readFile(configPath, 'utf-8'))
Expand Down
28 changes: 11 additions & 17 deletions tools/e2e/setup.mjs
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
import { appendFileSync, existsSync, promises, readFileSync, writeFileSync } from 'fs'
import { appendFile, mkdtemp, readFile, rm, writeFile } from 'fs/promises'
import { tmpdir } from 'os'
import { join, sep } from 'path'
import { cwd, env } from 'process'
import { fileURLToPath } from 'url'

import del from 'del'
import execa from 'execa'
import getPort from 'get-port'
import verdaccio from 'verdaccio'

// TODO: remove this once `../../src/lib/fs.js` is an esm module as well
const rmdirRecursiveAsync = async (path) => {
await del(path, { force: true })
}

const { mkdtemp } = promises
import { fileExistsAsync } from '../../src/lib/fs.cjs'

// eslint-disable-next-line no-magic-numbers
const VERDACCIO_TIMEOUT_MILLISECONDS = 60 * 1000
const START_PORT_RANGE = 5000
const END_PORT_RANGE = 5000
Expand Down Expand Up @@ -70,7 +63,7 @@ export const startRegistry = async () => {
const storage = fileURLToPath(new URL('../../.verdaccio-storage', import.meta.url))

// Remove netlify-cli from the verdaccio storage because we are going to publish it in a second
await rmdirRecursiveAsync(join(storage, 'netlify-cli'))
await rm(join(storage, 'netlify-cli'), { force: true, recursive: true })

return new Promise((resolve, reject) => {
setTimeout(() => {
Expand Down Expand Up @@ -105,17 +98,17 @@ export const setup = async () => {
/** Cleans up everything */
const cleanup = async () => {
// remote temp folders
await rmdirRecursiveAsync(workspace)
await rm(workspace, { force: true, recursive: true })
}

env.npm_config_registry = url

try {
if (existsSync(npmrc)) {
backupNpmrc = readFileSync(npmrc, 'utf-8')
appendFileSync(npmrc, registryWithAuth)
if (await fileExistsAsync(npmrc)) {
backupNpmrc = await readFile(npmrc, 'utf-8')
await appendFile(npmrc, registryWithAuth)
} else {
writeFileSync(npmrc, registryWithAuth, 'utf-8')
await writeFile(npmrc, registryWithAuth, 'utf-8')
}

// publish the CLI package to our registry
Expand All @@ -137,10 +130,11 @@ ${error_ instanceof Error ? error_.message : error_}`,
)
} finally {
// restore .npmrc
// eslint-disable-next-line unicorn/prefer-ternary
if (backupNpmrc) {
writeFileSync(npmrc, backupNpmrc)
await writeFile(npmrc, backupNpmrc)
} else {
await rmdirRecursiveAsync(npmrc)
await rm(npmrc, { force: true, recursive: true })
}
}

Expand Down

1 comment on commit d3dfa17

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Package size: 249 MB

Please sign in to comment.