Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

fix: ensure directories are created before writing files #816

Merged
merged 1 commit into from
Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/runtimes/node/utils/zip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import pMap from 'p-map'
import unixify from 'unixify'

import { startZip, addZipFile, addZipContent, endZip, ZipArchive } from '../../../archive'
import { mkdirAndWriteFile } from '../../../utils/fs'

const pStat = promisify(fs.stat)
const pWriteFile = promisify(fs.writeFile)
Expand Down Expand Up @@ -79,7 +80,7 @@ const createDirectory = async function ({
const absoluteDestPath = join(functionFolder, normalizedDestPath)

if (rewrites.has(srcFile)) {
return pWriteFile(absoluteDestPath, rewrites.get(srcFile) as string)
return mkdirAndWriteFile(absoluteDestPath, rewrites.get(srcFile) as string)
}

return copyFile(srcFile, absoluteDestPath)
Expand Down
15 changes: 14 additions & 1 deletion src/utils/fs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { lstat, readdir, readFile, stat, unlink, writeFile } from 'fs'
import { format, join, parse, resolve } from 'path'
import { dirname, format, join, parse, resolve } from 'path'
import { promisify } from 'util'

import makeDir from 'make-dir'

import { nonNullable } from './non_nullable'

const pLstat = promisify(lstat)
Expand Down Expand Up @@ -86,6 +88,16 @@ const resolveFunctionsDirectories = (input: string | string[]) => {
return absoluteDirectories
}

const mkdirAndWriteFile: typeof pWriteFile = async (path, ...params) => {
if (typeof path === 'string') {
Copy link
Member Author

Choose a reason for hiding this comment

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

Checking that path is a string because it can also be a file descriptor.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is a file descriptor, shouldn't pWriteFile() be used instead of mkdirAndWriteFile() then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely, but we still need to account for the possibility that someone decides to use mkdirAndWriteFile with a file descriptor. If nothing else, TypeScript forces us to have this check since we're setting mkdirAndWriteFile to have the same type as pWriteFile.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

const directory = dirname(path)

await makeDir(directory)
eduardoboucas marked this conversation as resolved.
Show resolved Hide resolved
}

return pWriteFile(path, ...params)
}

export {
cachedLstat,
cachedReaddir,
Expand All @@ -99,5 +111,6 @@ export {
pStat as stat,
pWriteFile as writeFile,
pReadFile as readFile,
mkdirAndWriteFile,
}
export type { FsCache }
42 changes: 41 additions & 1 deletion tests/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,46 @@ testMany(
},
)

testMany(
'Can bundle functions with `.js` extension using ES Modules when `archiveType` is `none`',
['bundler_esbuild', 'bundler_nft', 'bundler_nft_transpile'],
async (options, t, variation) => {
const length = 4
const fixtureName = 'local-require-esm'
const opts = merge(options, {
archiveFormat: 'none',
basePath: `${FIXTURES_DIR}/${fixtureName}`,
featureFlags: { defaultEsModulesToEsbuild: false },
})
const { tmpDir } = await zipFixture(t, 'local-require-esm', {
length,
opts,
})

const func1 = () => require(join(tmpDir, 'function', 'function.js'))
const func2 = () => require(join(tmpDir, 'function_cjs', 'function_cjs.js'))
const func3 = () => require(join(tmpDir, 'function_export_only', 'function_export_only.js'))
const func4 = () => require(join(tmpDir, 'function_import_only', 'function_import_only.js'))

// Dynamic imports are not supported in Node <13.2.0.
if (semver.gte(nodeVersion, '13.2.0')) {
t.is(await func2()(), 0)
}

if (variation === 'bundler_nft') {
t.throws(func1)
t.throws(func3)
t.throws(func4)

return
}

t.is(func1().ZERO, 0)
t.is(typeof func3().howdy, 'string')
t.deepEqual(func4(), {})
},
)

testMany(
'Can bundle CJS functions that import ESM files with an `import()` expression',
['bundler_esbuild', 'bundler_nft', 'bundler_nft_transpile'],
Expand Down Expand Up @@ -1809,7 +1849,7 @@ test('The dynamic import runtime shim handles files in nested directories', asyn
t.throws(() => func('fr'))
})

test('The dynamic import runtime shim handles files in nested directories when using `archiveType: "none"`', async (t) => {
test('The dynamic import runtime shim handles files in nested directories when using `archiveFormat: "none"`', async (t) => {
const fixtureName = 'node-module-dynamic-import-4'
const { tmpDir } = await zipNode(t, fixtureName, {
opts: {
Expand Down