Skip to content

Commit

Permalink
Fix bugs when warning users using SQLite in Durable Objects in remote…
Browse files Browse the repository at this point in the history
… dev (#6699)
  • Loading branch information
joshthoward authored Sep 13, 2024
1 parent 9b6a74a commit 2507304
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 70 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-students-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix: Bugs when warning users using SQLite in Durable Objects in remote dev
17 changes: 9 additions & 8 deletions packages/wrangler/src/api/startDevWorker/ConfigController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
maskVars,
} from "../../dev";
import { getLocalPersistencePath } from "../../dev/get-local-persistence-path";
import { getClassNamesWhichUseSQLite } from "../../dev/validate-dev-props";
import { UserError } from "../../errors";
import { processExperimentalAssetsArg } from "../../experimental-assets";
import { logger } from "../../logger";
Expand Down Expand Up @@ -319,17 +320,17 @@ async function resolveConfig(
);
}

// TODO(do) support remote wrangler dev
const classNamesWhichUseSQLite = getClassNamesWhichUseSQLite(
resolved.migrations
);
if (
resolved.migrations?.some(
(m) =>
Array.isArray(m.new_sqlite_classes) && m.new_sqlite_classes.length > 0
) &&
resolved.dev?.remote
resolved.dev.remote &&
Array.from(classNamesWhichUseSQLite.values()).some((v) => v)
) {
throw new UserError(
"SQLite in Durable Objects is only supported in local mode."
);
logger.warn("SQLite in Durable Objects is only supported in local mode.");
}

return resolved;
}
export class ConfigController extends Controller<ConfigControllerEventMap> {
Expand Down
12 changes: 0 additions & 12 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -618,18 +618,6 @@ export async function startDev(args: StartDevOptions) {
);
}

if (
args.remote &&
config.migrations?.some(
(m) =>
Array.isArray(m.new_sqlite_classes) && m.new_sqlite_classes.length > 0
)
) {
throw new UserError(
"SQLite in Durable Objects is only supported in local mode."
);
}

const projectRoot = configPath && path.dirname(configPath);

const devEnv = new DevEnv();
Expand Down
16 changes: 15 additions & 1 deletion packages/wrangler/src/dev/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ import { openInspector } from "./inspect";
import { Local, maybeRegisterLocalWorker } from "./local";
import { Remote } from "./remote";
import { useEsbuild } from "./use-esbuild";
import { validateDevProps } from "./validate-dev-props";
import {
getClassNamesWhichUseSQLite,
validateDevProps,
} from "./validate-dev-props";
import type {
DevEnv,
ProxyData,
Expand Down Expand Up @@ -634,6 +637,17 @@ function DevSession(props: DevSessionProps) {
);
}

// TODO(do) support remote wrangler dev
const classNamesWhichUseSQLite = getClassNamesWhichUseSQLite(
props.migrations
);
if (
!props.local &&
Array.from(classNamesWhichUseSQLite.values()).some((v) => v)
) {
logger.warn("SQLite in Durable Objects is only supported in local mode.");
}

// this won't be called with props.experimentalDevEnv because useWorker is guarded with the same flag
const announceAndOnReady: typeof props.onReady = async (
finalIp,
Expand Down
50 changes: 1 addition & 49 deletions packages/wrangler/src/dev/miniflare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { UserError } from "../errors";
import { logger } from "../logger";
import { getSourceMappedString } from "../sourcemap";
import { updateCheck } from "../update-check";
import { getClassNamesWhichUseSQLite } from "./validate-dev-props";
import type { ServiceFetch } from "../api";
import type { Config } from "../config";
import type {
Expand Down Expand Up @@ -676,55 +677,6 @@ export function buildMiniflareBindingOptions(config: MiniflareBindingsConfig): {
};
}

function getClassNamesWhichUseSQLite(
migrations: Config["migrations"] | undefined
) {
const classNameToUseSQLite = new Map<string, boolean>();
(migrations || []).forEach((migration) => {
migration.deleted_classes?.forEach((deleted_class) => {
if (!classNameToUseSQLite.delete(deleted_class)) {
throw new UserError(
`Cannot apply deleted_classes migration to non-existent class ${deleted_class}`
);
}
});

migration.renamed_classes?.forEach(({ from, to }) => {
const useSQLite = classNameToUseSQLite.get(from);
if (useSQLite === undefined) {
throw new UserError(
`Cannot apply renamed_classes migration to non-existent class ${from}`
);
} else {
classNameToUseSQLite.delete(from);
classNameToUseSQLite.set(to, useSQLite);
}
});

migration.new_classes?.forEach((new_class) => {
if (classNameToUseSQLite.has(new_class)) {
throw new UserError(
`Cannot apply new_classes migration to existing class ${new_class}`
);
} else {
classNameToUseSQLite.set(new_class, false);
}
});

migration.new_sqlite_classes?.forEach((new_class) => {
if (classNameToUseSQLite.has(new_class)) {
throw new UserError(
`Cannot apply new_sqlite_classes migration to existing class ${new_class}`
);
} else {
classNameToUseSQLite.set(new_class, true);
}
});
});

return classNameToUseSQLite;
}

type PickTemplate<T, K extends string> = {
[P in keyof T & K]: T[P];
};
Expand Down
50 changes: 50 additions & 0 deletions packages/wrangler/src/dev/validate-dev-props.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { UserError } from "../errors";
import type { Config } from "../config";
import type { DevProps } from "./dev";

export function validateDevProps(props: Omit<DevProps, "host">) {
Expand Down Expand Up @@ -30,3 +31,52 @@ export function validateDevProps(props: Omit<DevProps, "host">) {
);
}
}

export function getClassNamesWhichUseSQLite(
migrations: Config["migrations"] | undefined
) {
const classNamesWhichUseSQLite = new Map<string, boolean>();
(migrations || []).forEach((migration) => {
migration.deleted_classes?.forEach((deleted_class) => {
if (!classNamesWhichUseSQLite.delete(deleted_class)) {
throw new UserError(
`Cannot apply deleted_classes migration to non-existent class ${deleted_class}`
);
}
});

migration.renamed_classes?.forEach(({ from, to }) => {
const useSQLite = classNamesWhichUseSQLite.get(from);
if (useSQLite === undefined) {
throw new UserError(
`Cannot apply renamed_classes migration to non-existent class ${from}`
);
} else {
classNamesWhichUseSQLite.delete(from);
classNamesWhichUseSQLite.set(to, useSQLite);
}
});

migration.new_classes?.forEach((new_class) => {
if (classNamesWhichUseSQLite.has(new_class)) {
throw new UserError(
`Cannot apply new_classes migration to existing class ${new_class}`
);
} else {
classNamesWhichUseSQLite.set(new_class, false);
}
});

migration.new_sqlite_classes?.forEach((new_class) => {
if (classNamesWhichUseSQLite.has(new_class)) {
throw new UserError(
`Cannot apply new_sqlite_classes migration to existing class ${new_class}`
);
} else {
classNamesWhichUseSQLite.set(new_class, true);
}
});
});

return classNamesWhichUseSQLite;
}

0 comments on commit 2507304

Please sign in to comment.