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

Jump to bottom

chore: refactor to use fs.promises.rm() #54076

Merged
merged 3 commits into from Aug 16, 2023
Merged

chore: refactor to use fs.promises.rm() #54076

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edcfbfa
chore: refactor to use `fs.promises.rm()`
styfle Aug 15, 2023
cb775ee
lint
styfle Aug 15, 2023
ad138b4
Merge branch 'canary' into refactor-rm-recursive-force-usage
kodiakhq[bot] Aug 16, 2023
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: 1 addition & 2 deletions packages/next/src/build/index.ts
View file
Open in desktop
Expand Up @@ -2989,8 +2989,7 @@ export default async function build(
}

// remove temporary export folder
await recursiveDelete(exportOptions.outdir)
await fs.rmdir(exportOptions.outdir)
await fs.rm(exportOptions.outdir, { recursive: true, force: true })
Copy link
Member Author

Choose a reason for hiding this comment

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

@ijjk I was trying to figure out why this was using two deletes and it looks like this goes back to 2019:

Does the fs.rm() solution solve this use case now?

await fs.writeFile(
manifestPath,
JSON.stringify(pagesManifest, null, 2),
Expand Down
3 changes: 1 addition & 2 deletions packages/next/src/build/utils.ts
View file
Open in desktop
Expand Up @@ -53,7 +53,6 @@ import {
} from '../server/load-components'
import { trace } from '../trace'
import { setHttpClientAndAgentOptions } from '../server/setup-http-agent-env'
import { recursiveDelete } from '../lib/recursive-delete'
import { Sema } from 'next/dist/compiled/async-sema'
import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
Expand Down Expand Up @@ -1809,7 +1808,7 @@ export async function copyTracedFiles(
moduleType = packageJson.type === 'module'
} catch {}
const copiedFiles = new Set()
await recursiveDelete(outputPath)
await fs.rm(outputPath, { recursive: true, force: true })

async function handleTraceFiles(traceFilePath: string) {
const traceData = JSON.parse(await fs.readFile(traceFilePath, 'utf8')) as {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/export/index.ts
View file
Open in desktop
Expand Up @@ -354,7 +354,7 @@ export default async function exportApp(
)
}

await recursiveDelete(join(outDir))
await promises.rm(outDir, { recursive: true, force: true })
await promises.mkdir(join(outDir, '_next', buildId), { recursive: true })

writeFileSync(
Expand Down
4 changes: 1 addition & 3 deletions packages/next/src/lib/verify-partytown-setup.ts
View file
Open in desktop
Expand Up @@ -8,7 +8,6 @@ import {
} from './has-necessary-dependencies'
import { fileExists, FileType } from './file-exists'
import { FatalError } from './fatal-error'
import { recursiveDelete } from './recursive-delete'
import * as Log from '../build/output/log'
import { getPkgManager } from './helpers/get-pkg-manager'

Expand Down Expand Up @@ -50,8 +49,7 @@ async function copyPartytownStaticFiles(
)

if (hasPartytownLibDir) {
await recursiveDelete(partytownLibDir)
await promises.rmdir(partytownLibDir)
await promises.rm(partytownLibDir, { recursive: true, force: true })
Copy link
Member Author

@styfle styfle Aug 15, 2023 •
edited

Choose a reason for hiding this comment

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

Similarly here, @housseindjirdeh was there a reason for both recursiveDelete() and promises.rmdir()? It was added in:

And does the new promises.rm() solve this use case?

}

const { copyLibFiles } = await Promise.resolve(
Expand Down
11 changes: 2 additions & 9 deletions packages/next/src/server/image-optimizer.ts
View file
Open in desktop
Expand Up @@ -99,15 +99,8 @@ async function writeToCacheDir(
) {
const filename = join(dir, `${maxAge}.${expireAt}.${etag}.${extension}`)

// Added in: v14.14.0 https://nodejs.org/api/fs.html#fspromisesrmpath-options
// attempt cleaning up existing stale cache
if ((promises as any).rm) {
await (promises as any)
.rm(dir, { force: true, recursive: true })
.catch(() => {})
} else {
await promises.rmdir(dir, { recursive: true }).catch(() => {})
}
await promises.rm(dir, { recursive: true, force: true }).catch(() => {})

await promises.mkdir(dir, { recursive: true })
await promises.writeFile(filename, buffer)
}
Expand Down