From fac2f9924ebf8c3d975998bbc3dd6f3f0d67ca10 Mon Sep 17 00:00:00 2001 From: Brendan Coll Date: Thu, 14 Dec 2023 11:52:44 +0000 Subject: [PATCH] fix: ensure source mapped locations correct on Windows Use `@cspotcode/source-map-support` instead of `source-map-support`. This has some additional fixes for resolving paths on Windows, and is used by `ts-node`. --- packages/miniflare/package.json | 2 +- .../src/plugins/core/errors/index.ts | 4 +-- .../src/plugins/core/errors/sourcemap.ts | 20 ++++++----- .../test/plugins/core/errors/index.spec.ts | 20 +++++++---- .../miniflare/test/test-shared/asserts.ts | 4 +-- packages/wrangler/package.json | 2 +- packages/wrangler/scripts/deps.ts | 2 ++ packages/wrangler/src/sourcemap.ts | 9 ++--- pnpm-lock.yaml | 36 +++++++------------ 9 files changed, 51 insertions(+), 48 deletions(-) diff --git a/packages/miniflare/package.json b/packages/miniflare/package.json index f1539c7dee8f..fd490e3d1131 100644 --- a/packages/miniflare/package.json +++ b/packages/miniflare/package.json @@ -41,12 +41,12 @@ "miniflare": "bootstrap.js" }, "dependencies": { + "@cspotcode/source-map-support": "0.8.1", "acorn": "^8.8.0", "acorn-walk": "^8.2.0", "capnp-ts": "^0.7.0", "exit-hook": "^2.2.1", "glob-to-regexp": "^0.4.1", - "source-map-support": "0.5.21", "stoppable": "^1.1.0", "undici": "^5.22.1", "workerd": "1.20231030.0", diff --git a/packages/miniflare/src/plugins/core/errors/index.ts b/packages/miniflare/src/plugins/core/errors/index.ts index 97d65580c703..1ba5ebea7b7a 100644 --- a/packages/miniflare/src/plugins/core/errors/index.ts +++ b/packages/miniflare/src/plugins/core/errors/index.ts @@ -1,7 +1,7 @@ import fs from "fs"; import path from "path"; import { fileURLToPath } from "url"; -import type { UrlAndMap } from "source-map-support"; +import type { UrlAndMap } from "@cspotcode/source-map-support"; import { z } from "zod"; import { Request, Response } from "../../../http"; import { Log } from "../../../shared"; @@ -49,7 +49,7 @@ interface SourceFile { } // Try to read a file from the file-system, returning undefined if not found -function maybeGetDiskFile(filePath: string): SourceFile | undefined { +function maybeGetDiskFile(filePath: string): Required | undefined { try { const contents = fs.readFileSync(filePath, "utf8"); return { path: filePath, contents }; diff --git a/packages/miniflare/src/plugins/core/errors/sourcemap.ts b/packages/miniflare/src/plugins/core/errors/sourcemap.ts index 4f0bd5c989ea..5bc29fc032dc 100644 --- a/packages/miniflare/src/plugins/core/errors/sourcemap.ts +++ b/packages/miniflare/src/plugins/core/errors/sourcemap.ts @@ -1,5 +1,5 @@ import assert from "assert"; -import type { Options } from "source-map-support"; +import type { Options } from "@cspotcode/source-map-support"; import { parseStack } from "./callsite"; const sourceMapInstallBaseOptions: Options = { @@ -8,6 +8,7 @@ const sourceMapInstallBaseOptions: Options = { handleUncaughtExceptions: false, // Don't hook Node `require` function hookRequire: false, + redirectConflictingLibrary: false, // Make sure we're using fresh copies of files (i.e. between `setOptions()`) emptyCacheBetweenOperations: true, @@ -36,10 +37,10 @@ export function getSourceMapper(): SourceMapper { // always updated, load a fresh copy, by resetting then restoring the // `require` cache. - const originalSupport = require.cache["source-map-support"]; - delete require.cache["source-map-support"]; - const support: typeof import("source-map-support") = require("source-map-support"); - require.cache["source-map-support"] = originalSupport; + const originalSupport = require.cache["@cspotcode/source-map-support"]; + delete require.cache["@cspotcode/source-map-support"]; + const support: typeof import("@cspotcode/source-map-support") = require("@cspotcode/source-map-support"); + require.cache["@cspotcode/source-map-support"] = originalSupport; const originalPrepareStackTrace = Error.prepareStackTrace; support.install(sourceMapInstallBaseOptions); @@ -50,10 +51,13 @@ export function getSourceMapper(): SourceMapper { sourceMapper = (retrieveSourceMap, error) => { support.install({ ...sourceMapInstallBaseOptions, - retrieveFile(file: string): string | null { + retrieveFile(_file: string): string { // `retrieveFile` should only be called by the default implementation of - // `retrieveSourceMap`, so we shouldn't need to implement it - assert.fail(`Unexpected retrieveFile(${JSON.stringify(file)}) call`); + // `retrieveSourceMap`, which will only be called if `retrieveSourceMap` + // failed. In that case, return an empty string so the default + // implementation fails too (can't find `// sourceMappingURL` comment + // in empty string). + return ""; }, retrieveSourceMap, }); diff --git a/packages/miniflare/test/plugins/core/errors/index.spec.ts b/packages/miniflare/test/plugins/core/errors/index.spec.ts index 4e018fbd799e..f06a39d76c5f 100644 --- a/packages/miniflare/test/plugins/core/errors/index.spec.ts +++ b/packages/miniflare/test/plugins/core/errors/index.spec.ts @@ -1,14 +1,14 @@ import assert from "assert"; import fs from "fs/promises"; import path from "path"; -import { fileURLToPath } from "url"; +import { fileURLToPath, pathToFileURL } from "url"; import test from "ava"; import Protocol from "devtools-protocol"; import esbuild from "esbuild"; import { DeferredPromise, Log, LogLevel, Miniflare, fetch } from "miniflare"; import type { RawSourceMap } from "source-map"; import NodeWebSocket from "ws"; -import { escapeRegexp, useTmp } from "../../../test-shared"; +import { escapeRegexpComponent, useTmp } from "../../../test-shared"; const FIXTURES_PATH = path.resolve( __dirname, @@ -26,6 +26,12 @@ const MODULES_ENTRY_PATH = path.join(FIXTURES_PATH, "modules.ts"); const DEP_ENTRY_PATH = path.join(FIXTURES_PATH, "nested/dep.ts"); const REDUCE_PATH = path.join(FIXTURES_PATH, "reduce.ts"); +function pathOrUrlRegexp(filePath: string): `(${string}|${string})` { + return `(${escapeRegexpComponent(filePath)}|${escapeRegexpComponent( + pathToFileURL(filePath).href + )})`; +} + test("source maps workers", async (t) => { // Build fixtures const tmp = await useTmp(t); @@ -135,8 +141,8 @@ addEventListener("fetch", (event) => { let error = await t.throwsAsync(mf.dispatchFetch("http://localhost"), { message: "unnamed", }); - const serviceWorkerEntryRegexp = escapeRegexp( - `${SERVICE_WORKER_ENTRY_PATH}:6:16` + const serviceWorkerEntryRegexp = new RegExp( + `${pathOrUrlRegexp(SERVICE_WORKER_ENTRY_PATH)}:6:16` ); t.regex(String(error?.stack), serviceWorkerEntryRegexp); error = await t.throwsAsync(mf.dispatchFetch("http://localhost/a"), { @@ -148,7 +154,9 @@ addEventListener("fetch", (event) => { error = await t.throwsAsync(mf.dispatchFetch("http://localhost/b"), { message: "b", }); - const modulesEntryRegexp = escapeRegexp(`${MODULES_ENTRY_PATH}:5:17`); + const modulesEntryRegexp = new RegExp( + `${pathOrUrlRegexp(MODULES_ENTRY_PATH)}:5:17` + ); t.regex(String(error?.stack), modulesEntryRegexp); error = await t.throwsAsync(mf.dispatchFetch("http://localhost/c"), { message: "c", @@ -174,7 +182,7 @@ addEventListener("fetch", (event) => { instanceOf: TypeError, message: "Dependency error", }); - const nestedRegexp = escapeRegexp(`${DEP_ENTRY_PATH}:4:16`); + const nestedRegexp = new RegExp(`${pathOrUrlRegexp(DEP_ENTRY_PATH)}:4:16`); t.regex(String(error?.stack), nestedRegexp); // Check source mapping URLs rewritten diff --git a/packages/miniflare/test/test-shared/asserts.ts b/packages/miniflare/test/test-shared/asserts.ts index b44d95bdf392..140765debc44 100644 --- a/packages/miniflare/test/test-shared/asserts.ts +++ b/packages/miniflare/test/test-shared/asserts.ts @@ -18,9 +18,9 @@ export function isWithin( ); } -export function escapeRegexp(value: string): RegExp { +export function escapeRegexpComponent(value: string): string { // From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions#escaping - return new RegExp(value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")); + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } export function flaky( diff --git a/packages/wrangler/package.json b/packages/wrangler/package.json index 0fb0372e34cb..6c731fbe85ff 100644 --- a/packages/wrangler/package.json +++ b/packages/wrangler/package.json @@ -102,6 +102,7 @@ }, "dependencies": { "@cloudflare/kv-asset-handler": "^0.2.0", + "@cspotcode/source-map-support": "0.8.1", "@esbuild-plugins/node-globals-polyfill": "^0.2.3", "@esbuild-plugins/node-modules-polyfill": "^0.2.2", "blake3-wasm": "^2.1.5", @@ -113,7 +114,6 @@ "resolve.exports": "^2.0.2", "selfsigned": "^2.0.1", "source-map": "0.6.1", - "source-map-support": "0.5.21", "xxhash-wasm": "^1.0.1" }, "devDependencies": { diff --git a/packages/wrangler/scripts/deps.ts b/packages/wrangler/scripts/deps.ts index 493a7c313070..d0b54c72ed7a 100644 --- a/packages/wrangler/scripts/deps.ts +++ b/packages/wrangler/scripts/deps.ts @@ -12,6 +12,8 @@ export const EXTERNAL_DEPENDENCIES = [ // todo - bundle miniflare too "selfsigned", "source-map", + // CommonJS module using `module.require()` which isn't provided by `esbuild` + "@cspotcode/source-map-support", "@esbuild-plugins/node-globals-polyfill", "@esbuild-plugins/node-modules-polyfill", "chokidar", diff --git a/packages/wrangler/src/sourcemap.ts b/packages/wrangler/src/sourcemap.ts index 6dcfb06e966c..ebce3abe67b6 100644 --- a/packages/wrangler/src/sourcemap.ts +++ b/packages/wrangler/src/sourcemap.ts @@ -15,11 +15,11 @@ function getSourceMappingPrepareStackTrace(): NonNullable< // affecting external user's use of this package, and so // `Error.prepareStackTrace` is always updated, load a fresh copy, by // resetting then restoring the `require` cache. - const originalSupport = require.cache["source-map-support"]; - delete require.cache["source-map-support"]; + const originalSupport = require.cache["@cspotcode/source-map-support"]; + delete require.cache["@cspotcode/source-map-support"]; // eslint-disable-next-line @typescript-eslint/consistent-type-imports,@typescript-eslint/no-var-requires - const support: typeof import("source-map-support") = require("source-map-support"); - require.cache["source-map-support"] = originalSupport; + const support: typeof import("@cspotcode/source-map-support") = require("@cspotcode/source-map-support"); + require.cache["@cspotcode/source-map-support"] = originalSupport; const originalPrepareStackTrace = Error.prepareStackTrace; support.install({ @@ -28,6 +28,7 @@ function getSourceMappingPrepareStackTrace(): NonNullable< handleUncaughtExceptions: false, // Don't hook Node `require` function hookRequire: false, + redirectConflictingLibrary: false, // Make sure we're using fresh copies of files each time we source map emptyCacheBetweenOperations: true, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 47882e429c39..676eb205bc91 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -782,6 +782,9 @@ importers: packages/miniflare: dependencies: + '@cspotcode/source-map-support': + specifier: 0.8.1 + version: 0.8.1 acorn: specifier: ^8.8.0 version: 8.10.0 @@ -797,9 +800,6 @@ importers: glob-to-regexp: specifier: ^0.4.1 version: 0.4.1 - source-map-support: - specifier: 0.5.21 - version: 0.5.21 stoppable: specifier: ^1.1.0 version: 1.1.0 @@ -1234,6 +1234,9 @@ importers: '@cloudflare/kv-asset-handler': specifier: ^0.2.0 version: 0.2.0 + '@cspotcode/source-map-support': + specifier: 0.8.1 + version: 0.8.1 '@esbuild-plugins/node-globals-polyfill': specifier: ^0.2.3 version: 0.2.3(esbuild@0.17.19) @@ -1267,9 +1270,6 @@ importers: source-map: specifier: 0.6.1 version: 0.6.1 - source-map-support: - specifier: 0.5.21 - version: 0.5.21 xxhash-wasm: specifier: ^1.0.1 version: 1.0.1 @@ -1563,7 +1563,7 @@ packages: engines: {node: '>=6.0.0'} dependencies: '@jridgewell/gen-mapping': 0.1.1 - '@jridgewell/trace-mapping': 0.3.17 + '@jridgewell/trace-mapping': 0.3.19 dev: true /@ava/typescript@4.1.0: @@ -1664,7 +1664,7 @@ packages: dependencies: '@babel/types': 7.22.19 '@jridgewell/gen-mapping': 0.3.2 - '@jridgewell/trace-mapping': 0.3.17 + '@jridgewell/trace-mapping': 0.3.19 jsesc: 2.5.2 dev: true @@ -1674,7 +1674,7 @@ packages: dependencies: '@babel/types': 7.22.5 '@jridgewell/gen-mapping': 0.3.2 - '@jridgewell/trace-mapping': 0.3.17 + '@jridgewell/trace-mapping': 0.3.19 jsesc: 2.5.2 dev: true @@ -3772,7 +3772,6 @@ packages: engines: {node: '>=12'} dependencies: '@jridgewell/trace-mapping': 0.3.9 - dev: true /@emotion/hash@0.9.1: resolution: {integrity: sha512-gJB6HLm5rYwSLI6PQa+X1t5CFGrv1J1TWG+sOyMCeKz2ojaj6Fnl/rZEspogG+cvqbt4AE/2eIyD2QfLKTBNlQ==} @@ -5188,33 +5187,21 @@ packages: dependencies: '@jridgewell/set-array': 1.1.0 '@jridgewell/sourcemap-codec': 1.4.15 - '@jridgewell/trace-mapping': 0.3.17 + '@jridgewell/trace-mapping': 0.3.19 dev: true /@jridgewell/resolve-uri@3.1.0: resolution: {integrity: sha512-F2msla3tad+Mfht5cJq7LSXcdudKTWCVYUgw6pLFOOHSTtZlj6SWNYAp+AhuqLmWdBO2X5hPrLcu8cVP8fy28w==} engines: {node: '>=6.0.0'} - dev: true /@jridgewell/set-array@1.1.0: resolution: {integrity: sha512-SfJxIxNVYLTsKwzB3MoOQ1yxf4w/E6MdkvTgrgAt1bfxjSrLUoHMKrDOykwN14q65waezZIdqDneUIPh4/sKxg==} engines: {node: '>=6.0.0'} dev: true - /@jridgewell/sourcemap-codec@1.4.14: - resolution: {integrity: sha512-XPSJHWmi394fuUuzDnGz1wiKqWfo1yXecHQMRf2l6hztTO+nPru658AyDngaBe7isIxEkRsPR3FZh+s7iVa4Uw==} - dev: true - /@jridgewell/sourcemap-codec@1.4.15: resolution: {integrity: sha512-eF2rxCRulEKXHTRiDrDy6erMYWqNw4LPdQ8UQA4huuxaQsVeRPFl2oM8oDGxMFhJUWZf9McpLtJasDDZb/Bpeg==} - /@jridgewell/trace-mapping@0.3.17: - resolution: {integrity: sha512-MCNzAp77qzKca9+W/+I0+sEpaUnZoeasnghNeVc41VZCEKaCH73Vq3BZZ/SzWIgrqE4H4ceI+p+b6C0mHf9T4g==} - dependencies: - '@jridgewell/resolve-uri': 3.1.0 - '@jridgewell/sourcemap-codec': 1.4.14 - dev: true - /@jridgewell/trace-mapping@0.3.19: resolution: {integrity: sha512-kf37QtfW+Hwx/buWGMPcR60iF9ziHa6r/CZJIHbmcm4+0qrXiVdxegAH0F6yddEVQ7zdkjcGCgCzUu+BcbhQxw==} dependencies: @@ -5227,7 +5214,6 @@ packages: dependencies: '@jridgewell/resolve-uri': 3.1.0 '@jridgewell/sourcemap-codec': 1.4.15 - dev: true /@jspm/core@2.0.1: resolution: {integrity: sha512-Lg3PnLp0QXpxwLIAuuJboLeRaIhrgJjeuh797QADg3xz8wGLugQOS5DpsE8A6i6Adgzf+bacllkKZG3J0tGfDw==} @@ -8249,6 +8235,7 @@ packages: /buffer-from@1.1.2: resolution: {integrity: sha512-E+XQCRwSbaaiChtv6k6Dwgc+bx+Bs6vuKJHHl5kox/BaKbhiXzqQOwK4cO22yElGp2OCmjwVhT3HmxgyPGnJfQ==} + dev: true /buffer@5.7.1: resolution: {integrity: sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ==} @@ -16871,6 +16858,7 @@ packages: dependencies: buffer-from: 1.1.2 source-map: 0.6.1 + dev: true /source-map-url@0.4.1: resolution: {integrity: sha512-cPiFOTLUKvJFIg4SKVScy4ilPPW6rFgMgfuZJPNoDuMs3nC1HbMUycBoJw77xFIp6z1UJQJOfx6C9GMH80DiTw==}