Skip to content

Commit

Permalink
fix: store temporary files in .wrangler (#4127)
Browse files Browse the repository at this point in the history
Previously, Wrangler used the OS's default temporary directory. On
Windows, this is usually on the `C:` drive. If your source code was
on a different drive, `esbuild` would generate invalid source maps
(source paths relative to the cwd were used verbatim in the output,
rather than being relative to sources root and the source map
location), breaking breakpoint debugging in VSCode. This change
ensures intermediate files are always written to the same drive as
sources. It also ensures unused bundle outputs are cleaned up between
`wrangler pages dev` reloads, and the same file path is used for
writing functions routes.

Closes #4052
  • Loading branch information
mrbbot authored Oct 26, 2023
1 parent be0c628 commit 3d55f96
Show file tree
Hide file tree
Showing 17 changed files with 222 additions and 96 deletions.
34 changes: 34 additions & 0 deletions .changeset/honest-tips-tie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"wrangler": patch
---

fix: store temporary files in `.wrangler`

As Wrangler builds your code, it writes intermediate files to a temporary
directory that gets cleaned up on exit. Previously, Wrangler used the OS's
default temporary directory. On Windows, this is usually on the `C:` drive.
If your source code was on a different drive, our bundling tool would generate
invalid source maps, breaking breakpoint debugging. This change ensures
intermediate files are always written to the same drive as sources. It also
ensures unused build outputs are cleaned up when running `wrangler pages dev`.

This change also means you no longer need to set `cwd` and
`resolveSourceMapLocations` in `.vscode/launch.json` when creating an `attach`
configuration for breakpoint debugging. Your `.vscode/launch.json` should now
look something like...

```jsonc
{
"configurations": [
{
"name": "Wrangler",
"type": "node",
"request": "attach",
"port": 9229,
// These can be omitted, but doing so causes silent errors in the runtime
"attachExistingChildren": false,
"autoAttachChildProcesses": false
}
]
}
```
3 changes: 1 addition & 2 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"check:lint": "eslint .",
"check:type": "tsc",
"clean": "rimraf wrangler-dist miniflare-dist emitted-types",
"dev": "pnpm run clean && concurrently -c black,blue --kill-others-on-fail false 'pnpm run bundle --watch' 'pnpm run check:type --watch --preserveWatchOutput'",
"dev": "pnpm run clean && concurrently -c black,blue --kill-others-on-fail false \"pnpm run bundle --watch\" \"pnpm run check:type --watch --preserveWatchOutput\"",
"emit-types": "tsc -p tsconfig.emit.json && node -r esbuild-register scripts/emit-types.ts",
"prepublishOnly": "SOURCEMAPS=false npm run build",
"start": "pnpm run bundle && cross-env NODE_OPTIONS=--enable-source-maps ./bin/wrangler.js",
Expand Down Expand Up @@ -194,7 +194,6 @@
"strip-ansi": "^7.0.1",
"supports-color": "^9.2.2",
"timeago.js": "^4.0.2",
"tmp-promise": "^3.0.3",
"ts-dedent": "^2.2.0",
"undici": "5.20.0",
"update-check": "^1.5.4",
Expand Down
11 changes: 7 additions & 4 deletions packages/wrangler/src/api/pages/deploy.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { existsSync, lstatSync, readFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve as resolvePath } from "node:path";
import { cwd } from "node:process";
import { File, FormData } from "undici";
Expand All @@ -20,6 +19,7 @@ import {
} from "../../pages/functions/buildWorker";
import { validateRoutes } from "../../pages/functions/routes-validation";
import { upload } from "../../pages/upload";
import { getPagesTmpDir } from "../../pages/utils";
import { validate } from "../../pages/validate";
import { createUploadWorkerBundleContents } from "./create-worker-bundle-contents";
import type { BundleResult } from "../../deployment-bundle/bundle";
Expand Down Expand Up @@ -154,15 +154,15 @@ export async function deploy({
const functionsDirectory =
customFunctionsDirectory || join(cwd(), "functions");
const routesOutputPath = !existsSync(join(directory, "_routes.json"))
? join(tmpdir(), `_routes-${Math.random()}.json`)
? join(getPagesTmpDir(), `_routes-${Math.random()}.json`)
: undefined;

// Routing configuration displayed in the Functions tab of a deployment in Dash
let filepathRoutingConfig: string | undefined;

if (!_workerJS && existsSync(functionsDirectory)) {
const outputConfigPath = join(
tmpdir(),
getPagesTmpDir(),
`functions-filepath-routing-config-${Math.random()}.json`
);

Expand Down Expand Up @@ -257,7 +257,10 @@ export async function deploy({
});
} else if (_workerJS) {
if (bundle) {
const outfile = join(tmpdir(), `./bundledWorker-${Math.random()}.mjs`);
const outfile = join(
getPagesTmpDir(),
`./bundledWorker-${Math.random()}.mjs`
);
workerBundle = await buildRawWorker({
workerScriptPath: _workerPath,
outfile,
Expand Down
9 changes: 6 additions & 3 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
import path from "node:path";
import { URLSearchParams } from "node:url";
import chalk from "chalk";
import tmp from "tmp-promise";
import { fetchListResult, fetchResult } from "../cfetch";
import { printBindings } from "../config";
import { bundleWorker } from "../deployment-bundle/bundle";
Expand All @@ -27,6 +26,7 @@ import { getMigrationsToUpload } from "../durable";
import { logger } from "../logger";
import { getMetricsUsageHeaders } from "../metrics";
import { ParseError } from "../parse";
import { getWranglerTmpDir } from "../paths";
import { getQueue, putConsumer } from "../queues/client";
import { getWorkersDevSubdomain } from "../routes";
import { syncAssets } from "../sites";
Expand Down Expand Up @@ -72,6 +72,7 @@ type Props = {
keepVars: boolean | undefined;
logpush: boolean | undefined;
oldAssetTtl: number | undefined;
projectRoot: string | undefined;
};

type RouteObject = ZoneIdRoute | ZoneNameRoute | CustomDomainRoute;
Expand Down Expand Up @@ -407,7 +408,8 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
);
}

const destination = props.outDir ?? (await tmp.dir({ unsafeCleanup: true }));
const destination =
props.outDir ?? getWranglerTmpDir(props.projectRoot, "deploy");
const envName = props.env ?? "production";

const start = Date.now();
Expand Down Expand Up @@ -507,6 +509,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
// This could potentially cause issues as we no longer have identical behaviour between dev and deploy?
targetConsumer: "deploy",
local: false,
projectRoot: props.projectRoot,
}
);

Expand Down Expand Up @@ -693,7 +696,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
if (typeof destination !== "string") {
// this means we're using a temp dir,
// so let's clean up before we proceed
await destination.cleanup();
destination.remove();
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/deploy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export async function deployHandler(

const configPath =
args.config || (args.script && findWranglerToml(path.dirname(args.script)));
const projectRoot = configPath && path.dirname(configPath);
const config = readConfig(configPath, args);
const entry = await getEntry(args, config, "deploy");
await metrics.sendMetricsEvent(
Expand Down Expand Up @@ -319,5 +320,6 @@ export async function deployHandler(
keepVars: args.keepVars,
logpush: args.logpush,
oldAssetTtl: args.oldAssetTtl,
projectRoot,
});
}
25 changes: 13 additions & 12 deletions packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import * as path from "node:path";
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
import * as esbuild from "esbuild";
import tmp from "tmp-promise";
import { getBasePath } from "../paths";
import { getBasePath, getWranglerTmpDir } from "../paths";
import { applyMiddlewareLoaderFacade } from "./apply-middleware";
import {
isBuildFailure,
Expand Down Expand Up @@ -85,6 +84,7 @@ export type BundleOptions = {
isOutfile?: boolean;
forPages?: boolean;
local: boolean;
projectRoot: string | undefined;
};

/**
Expand Down Expand Up @@ -122,15 +122,13 @@ export async function bundleWorker(
isOutfile,
forPages,
local,
projectRoot,
}: BundleOptions
): Promise<BundleResult> {
// We create a temporary directory for any one-off files we
// need to create. This is separate from the main build
// directory (`destination`).
const unsafeTmpDir = await tmp.dir({ unsafeCleanup: true });
// Make sure we resolve all files relative to the actual temporary directory,
// without symlinks, otherwise `esbuild` will generate invalid source maps.
const tmpDirPath = fs.realpathSync(unsafeTmpDir.path);
const tmpDir = getWranglerTmpDir(projectRoot, "bundle");

const entryFile = entry.file;

Expand Down Expand Up @@ -234,12 +232,9 @@ export async function bundleWorker(
// we need to extract that file to an accessible place before injecting
// it in, hence this code here.

const checkedFetchFileToInject = path.join(tmpDirPath, "checked-fetch.js");
const checkedFetchFileToInject = path.join(tmpDir.path, "checked-fetch.js");

if (checkFetch && !fs.existsSync(checkedFetchFileToInject)) {
fs.mkdirSync(tmpDirPath, {
recursive: true,
});
fs.writeFileSync(
checkedFetchFileToInject,
fs.readFileSync(
Expand All @@ -264,7 +259,7 @@ export async function bundleWorker(
) {
const result = await applyMiddlewareLoaderFacade(
entry,
tmpDirPath,
tmpDir.path,
middlewareToLoad,
doBindings
);
Expand Down Expand Up @@ -365,10 +360,16 @@ export async function bundleWorker(
}

stop = async function () {
tmpDir.remove();
await ctx.dispose();
};
} else {
result = await esbuild.build(buildOptions);
// Even when we're not watching, we still want some way of cleaning up the
// temporary directory when we don't need it anymore
stop = async function () {
tmpDir.remove();
};
}
} catch (e) {
if (!legacyNodeCompat && isBuildFailure(e))
Expand Down Expand Up @@ -405,7 +406,7 @@ export async function bundleWorker(
stop,
sourceMapPath,
sourceMapMetadata: {
tmpDir: tmpDirPath,
tmpDir: tmpDir.path,
entryDirectory: entry.directory,
},
};
Expand Down
4 changes: 4 additions & 0 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ export async function startDev(args: StartDevOptions) {
const configPath =
args.config ||
(args.script && findWranglerToml(path.dirname(args.script)));
const projectRoot = configPath && path.dirname(configPath);
let config = readConfig(configPath, args);

if (config.configPath) {
Expand Down Expand Up @@ -476,6 +477,7 @@ export async function startDev(args: StartDevOptions) {
firstPartyWorker={configParam.first_party_worker}
sendMetrics={configParam.send_metrics}
testScheduled={args.testScheduled}
projectRoot={projectRoot}
/>
);
}
Expand Down Expand Up @@ -520,6 +522,7 @@ export async function startApiDev(args: StartDevOptions) {

const configPath =
args.config || (args.script && findWranglerToml(path.dirname(args.script)));
const projectRoot = configPath && path.dirname(configPath);
const config = readConfig(configPath, args);

const {
Expand Down Expand Up @@ -616,6 +619,7 @@ export async function startApiDev(args: StartDevOptions) {
sendMetrics: configParam.send_metrics,
testScheduled: args.testScheduled,
disableDevRegistry: args.disableDevRegistry ?? false,
projectRoot,
});
}

Expand Down
34 changes: 11 additions & 23 deletions packages/wrangler/src/dev/dev.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { spawn } from "node:child_process";
import fs from "node:fs";
import * as path from "node:path";
import * as util from "node:util";
import { watch } from "chokidar";
Expand All @@ -9,7 +8,6 @@ import { Box, Text, useApp, useInput, useStdin } from "ink";
import React, { useEffect, useRef, useState } from "react";
import { useErrorHandler, withErrorBoundary } from "react-error-boundary";
import onExit from "signal-exit";
import tmp from "tmp-promise";
import { fetch } from "undici";
import { runCustomBuild } from "../deployment-bundle/run-custom-build";
import {
Expand All @@ -20,6 +18,7 @@ import {
} from "../dev-registry";
import { logger } from "../logger";
import openInBrowser from "../open-in-browser";
import { getWranglerTmpDir } from "../paths";
import { openInspector } from "./inspect";
import { Local } from "./local";
import { Remote } from "./remote";
Expand All @@ -31,6 +30,7 @@ import type { Entry } from "../deployment-bundle/entry";
import type { CfModule, CfWorkerInit } from "../deployment-bundle/worker";
import type { WorkerRegistry } from "../dev-registry";
import type { EnablePagesAssetsServiceBindingOptions } from "../miniflare-cli/types";
import type { EphemeralDirectory } from "../paths";
import type { AssetPaths } from "../sites";

/**
Expand Down Expand Up @@ -164,6 +164,7 @@ export type DevProps = {
firstPartyWorker: boolean | undefined;
sendMetrics: boolean | undefined;
testScheduled: boolean | undefined;
projectRoot: string | undefined;
};

export function DevImplementation(props: DevProps): JSX.Element {
Expand Down Expand Up @@ -248,7 +249,7 @@ type DevSessionProps = DevProps & {
function DevSession(props: DevSessionProps) {
useCustomBuild(props.entry, props.build);

const directory = useTmpDir();
const directory = useTmpDir(props.projectRoot);

const workerDefinitions = useDevRegistry(
props.name,
Expand Down Expand Up @@ -284,6 +285,7 @@ function DevSession(props: DevSessionProps) {
targetConsumer: "dev",
testScheduled: props.testScheduled ?? false,
experimentalLocal: props.experimentalLocal,
projectRoot: props.projectRoot,
});

// TODO(queues) support remote wrangler dev
Expand Down Expand Up @@ -376,37 +378,23 @@ function DevSession(props: DevSessionProps) {
);
}

export interface DirectorySyncResult {
name: string;
removeCallback: () => void;
}

function useTmpDir(): string | undefined {
function useTmpDir(projectRoot: string | undefined): string | undefined {
const [directory, setDirectory] = useState<string>();
const handleError = useErrorHandler();
useEffect(() => {
let dir: DirectorySyncResult | undefined;
let dir: EphemeralDirectory | undefined;
try {
// const tmpdir = path.resolve(".wrangler", "tmp");
// fs.mkdirSync(tmpdir, { recursive: true });
// dir = tmp.dirSync({ unsafeCleanup: true, tmpdir });
dir = tmp.dirSync({ unsafeCleanup: true }) as DirectorySyncResult;
// Make sure we resolve all files relative to the actual temporary
// directory, without symlinks, otherwise `esbuild` will generate invalid
// source maps.
const realpath = fs.realpathSync(dir.name);
setDirectory(realpath);
dir = getWranglerTmpDir(projectRoot, "dev");
setDirectory(dir.path);
return;
} catch (err) {
logger.error(
"Failed to create temporary directory to store built files."
);
handleError(err);
}
return () => {
dir?.removeCallback();
};
}, [handleError]);
return () => dir?.remove();
}, [projectRoot, handleError]);
return directory;
}

Expand Down
Loading

0 comments on commit 3d55f96

Please sign in to comment.