Skip to content

Commit

Permalink
Fix facade source maps on macOS
Browse files Browse the repository at this point in the history
When applying facades, we write intermediate files to a temporary
directory and then build them with `esbuild`. This outputs
intermediate source maps, which get merged in the final bundle step
to produce a final source map. On macOS, `os.tmpdir()` looks like
`/var/folders/.../.../T`. `/var` is a symlink to `/private/var`.
Unfortunately, source maps end up assuming files in `/var`, not
`/private/var`, leading to invalid stack traces like
`/private/Users/.../d1-beta-facade.js:101:12`.

This change uses `realpathSync` to fully-resolve all symlinks before
we pass paths containing the temporary directory to `esbuild` for
source map generation. We actually implemented this same change in
#2249 for the middleware loader facade, but not the rest.
  • Loading branch information
mrbbot committed Apr 4, 2023
1 parent 7e43a21 commit bd56f93
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ export async function bundleWorker(
// need to create. This is separate from the main build
// directory (`destination`).
const tmpDir = await tmp.dir({ unsafeCleanup: true });
// Make sure we resolve all files relative to the actual temporary directory,
// otherwise we'll have issues with source maps
const tmpDirPath = fs.realpathSync(tmpDir.path);

const entryDirectory = path.dirname(entry.file);
let moduleCollector = createModuleCollector({
Expand Down Expand Up @@ -216,10 +219,10 @@ 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(tmpDir.path, "checked-fetch.js");
const checkedFetchFileToInject = path.join(tmpDirPath, "checked-fetch.js");

if (checkFetch && !fs.existsSync(checkedFetchFileToInject)) {
fs.mkdirSync(tmpDir.path, {
fs.mkdirSync(tmpDirPath, {
recursive: true,
});
fs.writeFileSync(
Expand Down Expand Up @@ -270,15 +273,15 @@ export async function bundleWorker(
// serve static assets
serveAssetsFromWorker &&
((currentEntry: Entry) => {
return applyStaticAssetFacade(currentEntry, tmpDir.path, assets);
return applyStaticAssetFacade(currentEntry, tmpDirPath, assets);
}),
// format errors nicely
// We use an env var here because we don't actually
// want to expose this to the user. It's only used internally to
// experiment with middleware as a teaching exercise.
process.env.FORMAT_WRANGLER_ERRORS === "true" &&
((currentEntry: Entry) => {
return applyFormatDevErrorsFacade(currentEntry, tmpDir.path);
return applyFormatDevErrorsFacade(currentEntry, tmpDirPath);
}),
// bind to other dev instances/service bindings
workerDefinitions &&
Expand All @@ -288,23 +291,23 @@ export async function bundleWorker(
((currentEntry: Entry) => {
return applyMultiWorkerDevFacade(
currentEntry,
tmpDir.path,
tmpDirPath,
services,
workerDefinitions
);
}),
// Simulate internal environment when using first party workers in dev
firstPartyWorkerDevFacade === true &&
((currentEntry: Entry) => {
return applyFirstPartyWorkerDevFacade(currentEntry, tmpDir.path);
return applyFirstPartyWorkerDevFacade(currentEntry, tmpDirPath);
}),

Array.isArray(betaD1Shims) &&
betaD1Shims.length > 0 &&
((currentEntry: Entry) => {
return applyD1BetaFacade(
currentEntry,
tmpDir.path,
tmpDirPath,
betaD1Shims,
local && !experimentalLocal,
doBindings
Expand All @@ -326,7 +329,7 @@ export async function bundleWorker(
((currentEntry: Entry) => {
return applyMiddlewareLoaderFacade(
currentEntry,
tmpDir.path,
tmpDirPath,
middlewareToLoad.filter(
// We dynamically filter the middleware depending on where we are bundling for
(m) =>
Expand Down Expand Up @@ -543,10 +546,6 @@ async function applyMiddlewareLoaderFacade(
// and then we load the middleware - this insertion and loading is
// different for each format.

// Make sure we resolve all files relative to the actual temporary directory,
// otherwise we'll have issues with source maps
tmpDirPath = fs.realpathSync(tmpDirPath);

const targetPathInsertion = path.join(
tmpDirPath,
"middleware-insertion.entry.js"
Expand Down

0 comments on commit bd56f93

Please sign in to comment.