Skip to content

Commit

Permalink
Revert "refactor: move projectRoot computation to config validation (#…
Browse files Browse the repository at this point in the history
…7415)"

This reverts commit 4f1a46e.
  • Loading branch information
petebacondarwin committed Dec 4, 2024
1 parent 6494817 commit c862267
Show file tree
Hide file tree
Showing 25 changed files with 93 additions and 110 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.
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
28 changes: 14 additions & 14 deletions packages/wrangler/src/__tests__/get-entry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,8 @@ 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 } from "../config/config";
import type { Entry } from "../deployment-bundle/entry";

function getConfig(): Config {
return {
...defaultWranglerConfig,
projectRoot: process.cwd(),
};
}

function normalize(entry: Entry): Entry {
const tmpDir = process.cwd();
const tmpDirName = path.basename(tmpDir);
Expand Down Expand Up @@ -45,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 @@ -64,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 @@ -85,10 +83,11 @@ describe("getEntry()", () => {
});
const entry = await getEntry(
{},
{ ...getConfig(), main: "index.ts" },
{ ...defaultWranglerConfig, main: "index.ts" },
"deploy"
);
expect(normalize(entry)).toMatchObject({
projectRoot: "/tmp/dir",
file: "/tmp/dir/index.ts",
moduleRoot: "/tmp/dir",
});
Expand All @@ -106,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 @@ -128,14 +128,14 @@ describe("getEntry()", () => {
const entry = await getEntry(
{},
{
...getConfig(),
...defaultWranglerConfig,
main: "src/index.ts",
configPath: "other-worker/wrangler.toml",
projectRoot: "other-worker",
},
"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
17 changes: 3 additions & 14 deletions packages/wrangler/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +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 to the configuration file (e.g. wrangler.toml/json), if one was provided. */
export interface ConfigFields<Dev extends RawDevConfig> {
configPath: string | undefined;

/** A worker's directory. Usually where the Wrangler configuration file is located */
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 @@ -333,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
4 changes: 2 additions & 2 deletions packages/wrangler/src/d1/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { requireAuth } from "../user";
import * as options from "./options";
import splitSqlQuery from "./splitter";
import { getDatabaseByNameOrBinding, getDatabaseInfoFromConfig } from "./utils";
import type { Config } from "../config";
import type { Config, ConfigFields, DevConfig, Environment } from "../config";
import type {
CommonYargsArgv,
StrictYargsOptionsToInterface,
Expand Down Expand Up @@ -197,7 +197,7 @@ export async function executeSql({
}: {
local: boolean | undefined;
remote: boolean | undefined;
config: Config;
config: ConfigFields<DevConfig> & Environment;
name: string;
shouldPrompt: boolean | undefined;
persistTo: string | undefined;
Expand Down
8 changes: 4 additions & 4 deletions packages/wrangler/src/d1/migrations/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { isNonInteractiveOrCI } from "../../is-interactive";
import { logger } from "../../logger";
import { DEFAULT_MIGRATION_PATH } from "../constants";
import { executeSql } from "../execute";
import type { Config } from "../../config";
import type { ConfigFields, DevConfig, Environment } from "../../config";
import type { QueryResult } from "../execute";
import type { Migration } from "../types";

Expand Down Expand Up @@ -57,7 +57,7 @@ export async function getUnappliedMigrations({
migrationsPath: string;
local: boolean | undefined;
remote: boolean | undefined;
config: Config;
config: ConfigFields<DevConfig> & Environment;
name: string;
persistTo: string | undefined;
preview: boolean | undefined;
Expand Down Expand Up @@ -92,7 +92,7 @@ type ListAppliedMigrationsProps = {
migrationsTableName: string;
local: boolean | undefined;
remote: boolean | undefined;
config: Config;
config: ConfigFields<DevConfig> & Environment;
name: string;
persistTo: string | undefined;
preview: boolean | undefined;
Expand Down Expand Up @@ -170,7 +170,7 @@ export const initMigrationsTable = async ({
migrationsTableName: string;
local: boolean | undefined;
remote: boolean | undefined;
config: Config;
config: ConfigFields<DevConfig> & Environment;
name: string;
persistTo: string | undefined;
preview: boolean | undefined;
Expand Down
Loading

0 comments on commit c862267

Please sign in to comment.