Skip to content

Commit

Permalink
[wrangler] chore: bump miniflare to 3.20231002.1 and fix `find_ad…
Browse files Browse the repository at this point in the history
…ditional_modules` source maps (#4107)

* chore: bump `miniflare` to `3.20231002.1`

* fix: use correct `//# sourceURL` with `find_additional_modules`

Previously, the automatically inserted `//# sourceURL` comments for
modules found using `find_additional_modules` referenced the
temporary directory, not the original source directory the modules
were from. This change makes sure they use the correct on-disk path,
ensuring source maps are resolved correctly, and breaking debugging
in IDEs works.
  • Loading branch information
mrbbot committed Oct 5, 2023
1 parent 7d20bdb commit 807ab93
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 58 deletions.
6 changes: 6 additions & 0 deletions .changeset/eleven-berries-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@cloudflare/pages-shared": patch
"wrangler": patch
---

chore: bump `miniflare` to [`3.20231002.1`](https://github.com/cloudflare/miniflare/releases/tag/v3.20231002.1)
2 changes: 1 addition & 1 deletion fixtures/pages-ws-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
},
"devDependencies": {
"@cloudflare/workers-tsconfig": "workspace:*",
"miniflare": "3.20231002.0",
"miniflare": "3.20231002.1",
"wrangler": "workspace:*",
"ws": "^8.8.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/pages-shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"test:ci": "vitest run"
},
"dependencies": {
"miniflare": "3.20231002.0"
"miniflare": "3.20231002.1"
},
"devDependencies": {
"@cloudflare/workers-tsconfig": "workspace:*",
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
"blake3-wasm": "^2.1.5",
"chokidar": "^3.5.3",
"esbuild": "0.17.19",
"miniflare": "3.20231002.0",
"miniflare": "3.20231002.1",
"nanoid": "^3.3.3",
"path-to-regexp": "^6.2.0",
"selfsigned": "^2.0.1",
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/api/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ export function parseRequestInput(
const forward = new Request(input, init);
const url = new URL(forward.url);
forward.headers.set("MF-Original-URL", url.toString());
forward.headers.set("MF-Disable-Pretty-Error", "true");
url.protocol = protocol;
url.hostname = readyAddress;
url.port = readyPort.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,13 @@ async function matchFiles(
if (!regexp.test(filePath)) {
continue;
}
const fileContent = await readFile(path.join(relativeTo, filePath));
const absoluteFilePath = path.join(relativeTo, filePath);
const fileContent = await readFile(absoluteFilePath);

const module = {
name: filePath,
content: fileContent,
filePath,
filePath: absoluteFilePath,
type: RuleTypeToModuleType[rule.type],
};

Expand Down
37 changes: 37 additions & 0 deletions packages/wrangler/src/deployment-bundle/source-url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import fs from "node:fs";
import { pathToFileURL } from "url";
import type { CfModule } from "./worker";

function withSourceURL(source: string, sourcePath: string) {
return `${source}\n//# sourceURL=${pathToFileURL(sourcePath)}`;
}

/**
* Adds `//# sourceURL` comments so V8 knows where source files are on disk.
* These URLs are returned in `Debugger.scriptParsed` events, ensuring inspector
* clients resolve source mapping URLs correctly. They also appear in stack
* traces, allowing users to click through to where errors are thrown.
*/
export function withSourceURLs(
entrypointPath: string,
modules: CfModule[]
): { entrypointSource: string; modules: CfModule[] } {
let entrypointSource = fs.readFileSync(entrypointPath, "utf8");
entrypointSource = withSourceURL(entrypointSource, entrypointPath);

modules = modules.map((module) => {
if (
module.filePath !== undefined &&
(module.type === "esm" || module.type === "commonjs")
) {
// `module.content` may be a `Buffer`
let newContent = module.content.toString();
newContent = withSourceURL(newContent, module.filePath);
return { ...module, content: newContent };
} else {
return module;
}
});

return { entrypointSource, modules };
}
11 changes: 8 additions & 3 deletions packages/wrangler/src/dev/miniflare.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import assert from "node:assert";
import { realpathSync } from "node:fs";
import { readFile } from "node:fs/promises";
import path from "node:path";
import {
Log,
Expand All @@ -11,6 +10,7 @@ import {
Miniflare,
} from "miniflare";
import { ModuleTypeToRuleType } from "../deployment-bundle/module-collection";
import { withSourceURLs } from "../deployment-bundle/source-url";
import { getHttpsOptions } from "../https-options";
import { logger } from "../logger";
import type { Config } from "../config";
Expand Down Expand Up @@ -152,24 +152,29 @@ async function buildSourceOptions(
const scriptPath = realpathSync(config.bundle.path);
if (config.format === "modules") {
const modulesRoot = path.dirname(scriptPath);
const { entrypointSource, modules } = withSourceURLs(
scriptPath,
config.bundle.modules
);
return {
modulesRoot,
modules: [
// Entrypoint
{
type: "ESModule",
path: scriptPath,
contents: await readFile(scriptPath, "utf-8"),
contents: entrypointSource,
},
// Misc (WebAssembly, etc, ...)
...config.bundle.modules.map((module) => ({
...modules.map((module) => ({
type: ModuleTypeToRuleType[module.type ?? "esm"],
path: path.resolve(modulesRoot, module.name),
contents: module.content,
})),
],
};
} else {
// Miniflare will handle adding `//# sourceURL` comments if they're missing
return { scriptPath };
}
}
Expand Down
35 changes: 4 additions & 31 deletions packages/wrangler/src/dev/remote.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { readFile } from "node:fs/promises";
import path from "node:path";
import { pathToFileURL } from "url";
import { Text } from "ink";
import SelectInput from "ink-select-input";
import React, { useState, useEffect, useRef } from "react";
import { useErrorHandler } from "react-error-boundary";
import { helpIfErrorIsSizeOrScriptStartup } from "../deploy/deploy";
import { printBundleSize } from "../deployment-bundle/bundle-reporter";
import { getBundleType } from "../deployment-bundle/bundle-type";
import { withSourceURLs } from "../deployment-bundle/source-url";
import { logger } from "../logger";
import { syncAssets } from "../sites";
import {
Expand Down Expand Up @@ -506,17 +505,6 @@ export async function getRemotePreviewToken(props: RemoteProps) {
});
}

/**
* Adds `//# sourceURL` comment so V8 knows where source files are on disk.
* This URL is returned in `Debugger.scriptParsed` events, ensuring inspector
* clients resolve source mapping URLs correctly. It also appears in stack
* traces, allowing users to click through to where errors are thrown. Note,
* Miniflare includes similar code, so we only need to do this in remote mode.
*/
function withSourceURL(source: string, sourcePath: string) {
return `${source}\n//# sourceURL=${pathToFileURL(sourcePath)}`;
}

async function createRemoteWorkerInit(props: {
bundle: EsbuildBundle;
modules: CfModule[];
Expand All @@ -532,9 +520,9 @@ async function createRemoteWorkerInit(props: {
compatibilityFlags: string[] | undefined;
usageModel: "bundled" | "unbound" | undefined;
}) {
const content = withSourceURL(
await readFile(props.bundle.path, "utf-8"),
props.bundle.path
const { entrypointSource: content, modules } = withSourceURLs(
props.bundle.path,
props.modules
);

// TODO: For Dev we could show the reporter message in the interactive box.
Expand All @@ -556,21 +544,6 @@ async function createRemoteWorkerInit(props: {
undefined
); // TODO: cancellable?

const modules = props.modules.map((module) => {
// If this is a JavaScript module with a `filePath`, add a `//# sourceURL`
// comment so source maps resolve correctly
if (
module.filePath !== undefined &&
(module.type === "esm" || module.type === "commonjs")
) {
// `module.content` may be a `Buffer`
let newContent = module.content.toString();
newContent = withSourceURL(newContent, module.filePath);
return { ...module, content: newContent };
} else {
return module;
}
});
if (assets.manifest) {
modules.push({
name: "__STATIC_CONTENT_MANIFEST",
Expand Down
Loading

0 comments on commit 807ab93

Please sign in to comment.