Skip to content

Commit

Permalink
Pbd/revert-project-root-refactoring (#7446)
Browse files Browse the repository at this point in the history
* fix up import-npm package lock

* Revert "refactor: remove missed redundant computation of `projectRoot` (#7421)"

This reverts commit bea6558.

* Revert "refactor: move projectRoot computation to config validation (#7415)"

This reverts commit 4f1a46e.

* Do not watch workflow tests in `test:ci` jobs

* add e2e test to prevent tmp directory regression
  • Loading branch information
petebacondarwin authored Dec 5, 2024
1 parent f5b9cd5 commit 9435af0
Show file tree
Hide file tree
Showing 27 changed files with 117 additions and 124 deletions.
8 changes: 8 additions & 0 deletions .changeset/proud-toes-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"wrangler": patch
---

fix: make sure Wrangler doesn't create a `.wrangler` tmp dir in the `functions/` folder of a Pages project

This regression was introduced in https://github.com/cloudflare/workers-sdk/pull/7415
and this change fixes it by reverting that change.
6 changes: 3 additions & 3 deletions fixtures/import-npm/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions packages/wrangler/e2e/pages-dev.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { existsSync } from "node:fs";
import path from "node:path";
import { setTimeout } from "node:timers/promises";
import getPort from "get-port";
import dedent from "ts-dedent";
Expand Down Expand Up @@ -482,6 +484,11 @@ describe.each([{ cmd: "wrangler pages dev" }])("Pages $cmd", ({ cmd }) => {

text = await fetchText(url);
expect(text).toBe("Updated Worker!");

// Ensure Wrangler doesn't write tmp files in the functions directory—regression test for https://github.com/cloudflare/workers-sdk/issues/7440
expect(
existsSync(path.join(helper.tmpPath, "functions/.wrangler"))
).toBeFalsy();
});

it("should support modifying dependencies during dev session (Functions)", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ function makeEsbuildBundle(testBundle: TestBundle): Bundle {
entrypointSource: "",
entry: {
file: "index.mjs",
projectRoot: "/virtual/",
format: "modules",
moduleRoot: "/virtual",
name: undefined,
Expand Down Expand Up @@ -232,6 +233,7 @@ describe("LocalRuntimeController", () => {
`,
entry: {
file: "esm/index.mjs",
projectRoot: "/virtual/",
format: "modules",
moduleRoot: "/virtual",
name: undefined,
Expand Down Expand Up @@ -345,6 +347,7 @@ describe("LocalRuntimeController", () => {
path: "/virtual/index.js",
entry: {
file: "index.js",
projectRoot: "/virtual/",
format: "service-worker",
moduleRoot: "/virtual",
name: undefined,
Expand Down
1 change: 0 additions & 1 deletion packages/wrangler/src/__tests__/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ describe("normalizeAndValidateConfig()", () => {
compatibility_date: undefined,
compatibility_flags: [],
configPath: undefined,
projectRoot: process.cwd(),
d1_databases: [],
vectorize: [],
hyperdrive: [],
Expand Down
14 changes: 7 additions & 7 deletions packages/wrangler/src/__tests__/find-additional-modules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
process.cwd(),
{
file: path.join(process.cwd(), "./index.js"),
projectRoot: process.cwd(),
format: "modules",
moduleRoot: process.cwd(),
exports: [],
Expand Down Expand Up @@ -73,9 +73,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
process.cwd(),
{
file: path.join(process.cwd(), "./index.js"),
projectRoot: process.cwd(),
format: "modules",
moduleRoot: process.cwd(),
exports: [],
Expand Down Expand Up @@ -107,9 +107,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
path.join(process.cwd(), "./src/nested"),
{
file: path.join(process.cwd(), "./src/nested/index.js"),
projectRoot: path.join(process.cwd(), "./src/nested"),
format: "modules",
// The default module root is dirname(file)
moduleRoot: path.join(process.cwd(), "./src/nested"),
Expand Down Expand Up @@ -142,9 +142,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
path.join(process.cwd(), "./src/nested"),
{
file: path.join(process.cwd(), "./src/nested/index.js"),
projectRoot: path.join(process.cwd(), "./src/nested"),
format: "modules",
// The default module root is dirname(file)
moduleRoot: path.join(process.cwd(), "./src"),
Expand Down Expand Up @@ -177,9 +177,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
path.join(process.cwd(), "./src/nested"),
{
file: path.join(process.cwd(), "./src/nested/index.js"),
projectRoot: path.join(process.cwd(), "./src/nested"),
format: "modules",
// The default module root is dirname(file)
moduleRoot: path.join(process.cwd(), "./src"),
Expand Down Expand Up @@ -212,9 +212,9 @@ describe("traverse module graph", () => {
);

const modules = await findAdditionalModules(
path.join(process.cwd(), "./src"),
{
file: path.join(process.cwd(), "./src/index.js"),
projectRoot: path.join(process.cwd(), "./src"),
format: "modules",
// The default module root is dirname(file)
moduleRoot: path.join(process.cwd(), "./src"),
Expand Down Expand Up @@ -247,9 +247,9 @@ describe("traverse module graph", () => {

await expect(
findAdditionalModules(
path.join(process.cwd(), "./src"),
{
file: path.join(process.cwd(), "./src/index.js"),
projectRoot: path.join(process.cwd(), "./src"),
format: "modules",
// The default module root is dirname(file)
moduleRoot: path.join(process.cwd(), "./src"),
Expand Down
39 changes: 23 additions & 16 deletions packages/wrangler/src/__tests__/get-entry.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
import path from "path";
import dedent from "ts-dedent";
import { normalizeAndValidateConfig } from "../config/validation";
import { defaultWranglerConfig } from "../config/config";
import { getEntry } from "../deployment-bundle/entry";
import { mockConsoleMethods } from "./helpers/mock-console";
import { runInTempDir } from "./helpers/run-in-tmp";
import { seed } from "./helpers/seed";
import type { Config, RawConfig } from "../config";
import type { Entry } from "../deployment-bundle/entry";

function getConfig(raw: RawConfig = {}, configPath?: string): Config {
return normalizeAndValidateConfig(raw, configPath, {}).config;
}

function normalize(entry: Entry): Entry {
const tmpDir = process.cwd();
const tmpDirName = path.basename(tmpDir);
Expand Down Expand Up @@ -42,8 +37,13 @@ describe("getEntry()", () => {
}
`,
});
const entry = await getEntry({ script: "index.ts" }, getConfig(), "deploy");
const entry = await getEntry(
{ script: "index.ts" },
defaultWranglerConfig,
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir",
file: "/tmp/dir/index.ts",
moduleRoot: "/tmp/dir",
});
Expand All @@ -61,10 +61,11 @@ describe("getEntry()", () => {
});
const entry = await getEntry(
{ script: "src/index.ts" },
getConfig(),
defaultWranglerConfig,
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir",
file: "/tmp/dir/src/index.ts",
moduleRoot: "/tmp/dir/src",
});
Expand All @@ -80,8 +81,13 @@ describe("getEntry()", () => {
}
`,
});
const entry = await getEntry({}, getConfig({ main: "index.ts" }), "deploy");
const entry = await getEntry(
{},
{ ...defaultWranglerConfig, main: "index.ts" },
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir",
file: "/tmp/dir/index.ts",
moduleRoot: "/tmp/dir",
});
Expand All @@ -99,10 +105,11 @@ describe("getEntry()", () => {
});
const entry = await getEntry(
{},
getConfig({ main: "src/index.ts" }),
{ ...defaultWranglerConfig, main: "src/index.ts" },
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir",
file: "/tmp/dir/src/index.ts",
moduleRoot: "/tmp/dir/src",
});
Expand All @@ -120,15 +127,15 @@ describe("getEntry()", () => {
});
const entry = await getEntry(
{},
getConfig(
{
main: "src/index.ts",
},
"other-worker/wrangler.toml"
),
{
...defaultWranglerConfig,
main: "src/index.ts",
configPath: "other-worker/wrangler.toml",
},
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir/other-worker",
file: "/tmp/dir/other-worker/src/index.ts",
moduleRoot: "/tmp/dir/other-worker/src",
});
Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/__tests__/navigator-user-agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ describe("defineNavigatorUserAgent is respected", () => {
await bundleWorker(
{
file: path.resolve("src/index.js"),
projectRoot: process.cwd(),
format: "modules",
moduleRoot: path.dirname(path.resolve("src/index.js")),
exports: [],
Expand Down Expand Up @@ -166,6 +167,7 @@ describe("defineNavigatorUserAgent is respected", () => {
await bundleWorker(
{
file: path.resolve("src/index.js"),
projectRoot: process.cwd(),
format: "modules",
moduleRoot: path.dirname(path.resolve("src/index.js")),
exports: [],
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/pages/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2855,7 +2855,7 @@ Failed to publish your Function. Got error: Uncaught TypeError: a is not a funct
});
});

describe("in Advanced Mode [_worker.js]", () => {
describe("in Advanced Mode [_worker,js]", () => {
it("should upload an Advanced Mode project", async () => {
// set up the directory of static files to upload.
mkdirSync("public");
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/api/startDevWorker/BundlerController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {

const entry: Entry = {
file: config.entrypoint,
projectRoot: config.projectRoot,
format: config.build.format,
moduleRoot: config.build.moduleRoot,
exports: config.build.exports,
};

const entryDirectory = path.dirname(config.entrypoint);
const moduleCollector = createModuleCollector({
projectRoot: config.projectRoot,
wrangler1xLegacyModuleReferences: getWrangler1xLegacyModuleReferences(
entryDirectory,
config.entrypoint
Expand All @@ -103,7 +103,6 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
).bindings;
const bundleResult: Omit<BundleResult, "stop"> = !config.build?.bundle
? await noBundleWorker(
config.projectRoot,
entry,
config.build.moduleRules,
this.#tmpDir.path
Expand Down Expand Up @@ -235,6 +234,7 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
assert(this.#tmpDir);
const entry: Entry = {
file: config.entrypoint,
projectRoot: config.projectRoot,
format: config.build.format,
moduleRoot: config.build.moduleRoot,
exports: config.build.exports,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ async function resolveConfig(
compatibilityDate: getDevCompatibilityDate(config, input.compatibilityDate),
compatibilityFlags: input.compatibilityFlags ?? config.compatibility_flags,
entrypoint: entry.file,
projectRoot: config.projectRoot,
projectRoot: entry.projectRoot,
bindings,
migrations: input.migrations ?? config.migrations,
sendMetrics: input.sendMetrics ?? config.send_metrics,
Expand Down
24 changes: 3 additions & 21 deletions packages/wrangler/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,32 +22,17 @@ import type { CamelCaseKey } from "yargs";
* - `@breaking`: the deprecation/optionality is a breaking change from Wrangler v1.
* - `@todo`: there's more work to be done (with details attached).
*/
export type Config = ComputedConfigFields &
ConfigFields<DevConfig> &
PagesConfigFields &
Environment;
export type Config = ConfigFields<DevConfig> & PagesConfigFields & Environment;

export type RawConfig = Partial<ConfigFields<RawDevConfig>> &
PagesConfigFields &
RawEnvironment &
DeprecatedConfigFields &
EnvironmentMap & { $schema?: string };

export interface ComputedConfigFields {
/**
* Path (relative to current working directory) of the configuration file (e.g. wrangler.toml/json), if one was provided.
*/
export interface ConfigFields<Dev extends RawDevConfig> {
configPath: string | undefined;

/**
* Absolute path to the Worker's directory.
*
* Will be the directory containing the Wrangler configuration file,
* or the current working directory otherwise.
*/
projectRoot: string;
}
export interface ConfigFields<Dev extends RawDevConfig> {
/**
* A boolean to enable "legacy" style wrangler environments (from Wrangler v1).
* These have been superseded by Services, but there may be projects that won't
Expand Down Expand Up @@ -340,11 +325,8 @@ export const defaultWranglerConfig: Config = {
/*====================================================*/
/* Fields supported by Workers only */
/*====================================================*/
/* COMPUTED CONFIG FIELDS */
configPath: undefined,
projectRoot: process.cwd(),

/* TOP-LEVEL ONLY FIELDS */
configPath: undefined,
legacy_env: true,
site: undefined,
legacy_assets: undefined,
Expand Down
5 changes: 2 additions & 3 deletions packages/wrangler/src/config/validation-pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ const supportedPagesConfigFields = [
"dev",
"mtls_certificates",
"browser",
"upload_source_maps",
// normalizeAndValidateConfig() sets the following values
// normalizeAndValidateConfig() sets this value
"configPath",
"projectRoot",
"upload_source_maps",
] as const;

export function validatePagesConfig(
Expand Down
3 changes: 0 additions & 3 deletions packages/wrangler/src/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,6 @@ export function normalizeAndValidateConfig(
// Process the top-level default environment configuration.
const config: Config = {
configPath,
projectRoot: path.resolve(
configPath !== undefined ? path.dirname(configPath) : process.cwd()
),
pages_build_output_dir: normalizeAndValidatePagesBuildOutputDir(
configPath,
rawConfig.pages_build_output_dir
Expand Down
Loading

0 comments on commit 9435af0

Please sign in to comment.