From 5eee0f40d79fa76ca58fd80c828e7d468e39b8bf Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:34:52 +0000 Subject: [PATCH 01/13] add experimental_readRawConfig and tests --- packages/wrangler/src/__tests__/configuration.test.ts | 1 + packages/wrangler/src/config/config-helpers.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts index 01d2b05185ef..0cf1b1a8c770 100644 --- a/packages/wrangler/src/__tests__/configuration.test.ts +++ b/packages/wrangler/src/__tests__/configuration.test.ts @@ -5,6 +5,7 @@ import { normalizeAndValidateConfig } from "../config/validation"; import { run } from "../experimental-flags"; import { normalizeString } from "./helpers/normalize"; import { runInTempDir } from "./helpers/run-in-tmp"; +import { writeWorkerSource } from "./helpers/write-worker-source"; import { writeWranglerConfig } from "./helpers/write-wrangler-config"; import type { ConfigFields, diff --git a/packages/wrangler/src/config/config-helpers.ts b/packages/wrangler/src/config/config-helpers.ts index a02b6a47bf26..a81fda4e4f39 100644 --- a/packages/wrangler/src/config/config-helpers.ts +++ b/packages/wrangler/src/config/config-helpers.ts @@ -27,6 +27,7 @@ export function resolveWranglerConfigPath({ export function findWranglerConfig( referencePath: string = process.cwd() ): string | undefined { + console.dir(referencePath); return ( findUpSync(`wrangler.json`, { cwd: referencePath }) ?? findUpSync(`wrangler.jsonc`, { cwd: referencePath }) ?? From 1b5d6a35e5ec5f52eecf02964858c7518d99c9d7 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:51:13 +0000 Subject: [PATCH 02/13] move printBindings out of /config/index.ts --- packages/wrangler/src/__tests__/configuration.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts index 0cf1b1a8c770..01d2b05185ef 100644 --- a/packages/wrangler/src/__tests__/configuration.test.ts +++ b/packages/wrangler/src/__tests__/configuration.test.ts @@ -5,7 +5,6 @@ import { normalizeAndValidateConfig } from "../config/validation"; import { run } from "../experimental-flags"; import { normalizeString } from "./helpers/normalize"; import { runInTempDir } from "./helpers/run-in-tmp"; -import { writeWorkerSource } from "./helpers/write-worker-source"; import { writeWranglerConfig } from "./helpers/write-wrangler-config"; import type { ConfigFields, From 2ae3bb56c370315e26733af62a5fb934d4970eb2 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 12 Dec 2024 18:13:07 +0000 Subject: [PATCH 03/13] add experimental_patchConfig --- packages/wrangler/src/config/patch-config.ts | 59 ++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 packages/wrangler/src/config/patch-config.ts diff --git a/packages/wrangler/src/config/patch-config.ts b/packages/wrangler/src/config/patch-config.ts new file mode 100644 index 000000000000..69c5899fd545 --- /dev/null +++ b/packages/wrangler/src/config/patch-config.ts @@ -0,0 +1,59 @@ +import TOML from "@iarna/toml"; +import { applyEdits, format, parse as JSONCParse, modify } from "jsonc-parser"; +import { parseTOML, readFileSync } from "../parse"; +import type { RawConfig } from "./config"; +import type { JSONPath } from "jsonc-parser"; + +export const experimental_patchConfig = ( + configPath: string, + patch: RawConfig +) => { + // will be json shaped + let raw = readFileSync(configPath); + + if (configPath.endsWith("toml")) { + // what if they have a # in a string...? + if (raw.includes("#")) { + return; + } else { + // toml -> js object -> json string -> edits -> js object -> toml + raw = JSON.stringify(parseTOML(raw)); + } + } else if (!(configPath.endsWith("jsonc") || configPath.endsWith("json"))) { + throw new Error("shouldn't get here?"); + } + + const patchPaths: JSONPath[] = []; + getPath(patch, patchPaths); + for (const patchPath of patchPaths) { + const value = patchPath.pop(); + const edit = modify(raw, patchPath, value, { isArrayInsertion: true }); + raw = applyEdits(raw, edit); + const formatEdit = format(raw, undefined, {}); + raw = applyEdits(raw, formatEdit); + } + + if (configPath.endsWith(".toml")) { + return TOML.stringify(JSONCParse(raw)); + } + return raw; +}; + +const getPath = ( + obj: RawConfig, + allPaths: JSONPath[], + prevPath: JSONPath = [] +) => { + for (const [k, v] of Object.entries(obj)) { + const currentPath = [...prevPath, k]; + if (Array.isArray(v)) { + v.forEach((x) => { + allPaths.push([...currentPath, -1, x]); + }); + } else if (typeof v === "object") { + getPath(v, allPaths, currentPath); + } else { + allPaths.push([...currentPath, v]); + } + } +}; From ec827f4ffe33525f36af3c8c19fb629b338200f8 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:40:43 +0000 Subject: [PATCH 04/13] fixups --- packages/wrangler/src/config/config-helpers.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/wrangler/src/config/config-helpers.ts b/packages/wrangler/src/config/config-helpers.ts index a81fda4e4f39..a02b6a47bf26 100644 --- a/packages/wrangler/src/config/config-helpers.ts +++ b/packages/wrangler/src/config/config-helpers.ts @@ -27,7 +27,6 @@ export function resolveWranglerConfigPath({ export function findWranglerConfig( referencePath: string = process.cwd() ): string | undefined { - console.dir(referencePath); return ( findUpSync(`wrangler.json`, { cwd: referencePath }) ?? findUpSync(`wrangler.jsonc`, { cwd: referencePath }) ?? From ca49d0d094f687334670a429ba3f8d840a3deb12 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 13 Dec 2024 17:54:04 +0000 Subject: [PATCH 05/13] add tests --- .../src/__tests__/patch-config.test.ts | 248 ++++++++++++++++++ packages/wrangler/src/config/patch-config.ts | 32 ++- 2 files changed, 266 insertions(+), 14 deletions(-) create mode 100644 packages/wrangler/src/__tests__/patch-config.test.ts diff --git a/packages/wrangler/src/__tests__/patch-config.test.ts b/packages/wrangler/src/__tests__/patch-config.test.ts new file mode 100644 index 000000000000..f72f7c22a8d0 --- /dev/null +++ b/packages/wrangler/src/__tests__/patch-config.test.ts @@ -0,0 +1,248 @@ +import { writeFileSync } from "node:fs"; +import dedent from "ts-dedent"; +import { experimental_patchConfig } from "../config/patch-config"; +import { runInTempDir } from "./helpers/run-in-tmp"; +import { writeWranglerConfig } from "./helpers/write-wrangler-config"; +import type { RawConfig } from "../config"; + +type TestCase = { + name: string; + original: RawConfig; + patch: RawConfig; + expectedToml: string; + expectedJson: string; +}; +const testCases: TestCase[] = [ + { + name: "add a binding", + original: {}, + patch: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV" + } + ] + } + `, + }, + { + name: "add a second binding of the same type", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + patch: { + kv_namespaces: [ + { + binding: "KV2", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV" + + [[kv_namespaces]] + binding = "KV2" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV" + }, + { + "binding": "KV2" + } + ] + } + `, + }, + { + name: "make multiple edits at the same time", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + d1_databases: [ + { + binding: "DB", + }, + ], + }, + patch: { + kv_namespaces: [ + { + binding: "KV2", + }, + { + binding: "KV3", + }, + ], + d1_databases: [ + { + binding: "DB2", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV" + + [[kv_namespaces]] + binding = "KV2" + + [[kv_namespaces]] + binding = "KV3" + + [[d1_databases]] + binding = "DB" + + [[d1_databases]] + binding = "DB2" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV" + }, + { + "binding": "KV2" + }, + { + "binding": "KV3" + } + ], + "d1_databases": [ + { + "binding": "DB" + }, + { + "binding": "DB2" + } + ] + } + `, + }, +]; + +describe("experimental_patchConfig()", () => { + runInTempDir(); + describe.each(["json", "toml"])("%s", (configType) => { + it.each(testCases)( + `$name`, + ({ original, patch, expectedJson, expectedToml }) => { + writeWranglerConfig( + original, + configType === "json" ? "./wrangler.json" : "./wrangler.toml" + ); + const result = experimental_patchConfig( + configType === "json" ? "./wrangler.json" : "./wrangler.toml", + patch + ); + expect(result).not.toBeFalsy(); + expect(result).toEqual( + `${configType === "json" ? expectedJson : expectedToml}` + ); + } + ); + }); + + describe("jsonc", () => { + it("preserves comments", () => { + const jsonc = ` + { + // a comment + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + // more comments! + "binding": "KV" + } + ], + "d1_databases": [ + /** + * multiline comment + */ + { + "binding": "DB" + } + ] + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const result = experimental_patchConfig( + "./wrangler.jsonc", + testCases[2].patch + ); + expect(result).not.toBeFalsy(); + expect(result).toMatchInlineSnapshot(` + "{ + // a comment + \\"compatibility_date\\": \\"2022-01-12\\", + \\"name\\": \\"test-name\\", + \\"kv_namespaces\\": [ + { + // more comments! + \\"binding\\": \\"KV\\" + }, + { + \\"binding\\": \\"KV2\\" + }, + { + \\"binding\\": \\"KV3\\" + } + ], + \\"d1_databases\\": [ + /** + * multiline comment + */ + { + \\"binding\\": \\"DB\\" + }, + { + \\"binding\\": \\"DB2\\" + } + ] + }" + `); + }); + }); +}); diff --git a/packages/wrangler/src/config/patch-config.ts b/packages/wrangler/src/config/patch-config.ts index 69c5899fd545..ce9aa137174b 100644 --- a/packages/wrangler/src/config/patch-config.ts +++ b/packages/wrangler/src/config/patch-config.ts @@ -5,38 +5,42 @@ import type { RawConfig } from "./config"; import type { JSONPath } from "jsonc-parser"; export const experimental_patchConfig = ( - configPath: string, + configPath: string | undefined, patch: RawConfig ) => { - // will be json shaped - let raw = readFileSync(configPath); + if (!configPath) { + return; + } + + let configString = readFileSync(configPath); if (configPath.endsWith("toml")) { - // what if they have a # in a string...? - if (raw.includes("#")) { + // the TOML parser we use does not preserve comments + if (configString.includes("#")) { return; } else { + // for simplicity, use the JSONC editor to make all edits // toml -> js object -> json string -> edits -> js object -> toml - raw = JSON.stringify(parseTOML(raw)); + configString = JSON.stringify(parseTOML(configString)); } - } else if (!(configPath.endsWith("jsonc") || configPath.endsWith("json"))) { - throw new Error("shouldn't get here?"); } const patchPaths: JSONPath[] = []; getPath(patch, patchPaths); for (const patchPath of patchPaths) { const value = patchPath.pop(); - const edit = modify(raw, patchPath, value, { isArrayInsertion: true }); - raw = applyEdits(raw, edit); - const formatEdit = format(raw, undefined, {}); - raw = applyEdits(raw, formatEdit); + const edit = modify(configString, patchPath, value, { + isArrayInsertion: true, + }); + configString = applyEdits(configString, edit); + const formatEdit = format(configString, undefined, {}); + configString = applyEdits(configString, formatEdit); } if (configPath.endsWith(".toml")) { - return TOML.stringify(JSONCParse(raw)); + return TOML.stringify(JSONCParse(configString)); } - return raw; + return configString; }; const getPath = ( From d6fae93908c76dd1c4eb96d05b2ee559df46f97c Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 13 Dec 2024 18:02:47 +0000 Subject: [PATCH 06/13] changeset --- .changeset/lovely-rats-live.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/lovely-rats-live.md diff --git a/.changeset/lovely-rats-live.md b/.changeset/lovely-rats-live.md new file mode 100644 index 000000000000..fb1a8a11e4e1 --- /dev/null +++ b/.changeset/lovely-rats-live.md @@ -0,0 +1,10 @@ +--- +"wrangler": patch +--- + +feat: add experimental_patchConfig() and experimental_readRawConfig() + +Adds two Wrangler APIs: + +1. experimental_readRawConfig() will find and read a config file +2. experimental_patchConfig() can add to a user's config file. It preserves comments if its a `wrangler.jsonc`. However, it is not suitable for `wrangler.toml` with comments as we cannot preserve comments on write. From 1536f834eb6c300a24807ff83120bd43fcf15e51 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:34:17 +0000 Subject: [PATCH 07/13] more tests --- .../src/__tests__/patch-config.test.ts | 100 +++++++++++++++++- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/packages/wrangler/src/__tests__/patch-config.test.ts b/packages/wrangler/src/__tests__/patch-config.test.ts index f72f7c22a8d0..258ba0ce1017 100644 --- a/packages/wrangler/src/__tests__/patch-config.test.ts +++ b/packages/wrangler/src/__tests__/patch-config.test.ts @@ -85,6 +85,86 @@ const testCases: TestCase[] = [ } `, }, + { + name: "add a new D1 binding when only KV bindings exist", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + patch: { + d1_databases: [ + { + binding: "DB", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV" + + [[d1_databases]] + binding = "DB" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV" + } + ], + "d1_databases": [ + { + "binding": "DB" + } + ] + } + `, + }, + { + name: "add a new field", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + patch: { + compatibility_flags: ["nodejs_compat"], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + compatibility_flags = [ "nodejs_compat" ] + + [[kv_namespaces]] + binding = "KV" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV" + } + ], + "compatibility_flags": [ + "nodejs_compat" + ] + } + `, + }, { name: "make multiple edits at the same time", original: { @@ -208,10 +288,22 @@ describe("experimental_patchConfig()", () => { } `; writeFileSync("./wrangler.jsonc", jsonc); - const result = experimental_patchConfig( - "./wrangler.jsonc", - testCases[2].patch - ); + const patch = { + kv_namespaces: [ + { + binding: "KV2", + }, + { + binding: "KV3", + }, + ], + d1_databases: [ + { + binding: "DB2", + }, + ], + }; + const result = experimental_patchConfig("./wrangler.jsonc", patch); expect(result).not.toBeFalsy(); expect(result).toMatchInlineSnapshot(` "{ From 2a05ea05e2d32201c45e9056313a6af32ee9ad01 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:58:29 +0000 Subject: [PATCH 08/13] pr feedback --- packages/wrangler/src/config/patch-config.ts | 37 ++++++++++++-------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/wrangler/src/config/patch-config.ts b/packages/wrangler/src/config/patch-config.ts index ce9aa137174b..d28128b7c7c6 100644 --- a/packages/wrangler/src/config/patch-config.ts +++ b/packages/wrangler/src/config/patch-config.ts @@ -1,23 +1,22 @@ +import { writeFileSync } from "fs"; import TOML from "@iarna/toml"; -import { applyEdits, format, parse as JSONCParse, modify } from "jsonc-parser"; -import { parseTOML, readFileSync } from "../parse"; +import { applyEdits, format, modify } from "jsonc-parser"; +import { parseJSONC, parseTOML, readFileSync } from "../parse"; import type { RawConfig } from "./config"; import type { JSONPath } from "jsonc-parser"; export const experimental_patchConfig = ( - configPath: string | undefined, + configPath: string, patch: RawConfig ) => { - if (!configPath) { - return; - } - let configString = readFileSync(configPath); if (configPath.endsWith("toml")) { // the TOML parser we use does not preserve comments if (configString.includes("#")) { - return; + throw new PatchConfigError( + "cannot patch .toml config if comments are present" + ); } else { // for simplicity, use the JSONC editor to make all edits // toml -> js object -> json string -> edits -> js object -> toml @@ -26,24 +25,26 @@ export const experimental_patchConfig = ( } const patchPaths: JSONPath[] = []; - getPath(patch, patchPaths); + getJSONPath(patch, patchPaths); for (const patchPath of patchPaths) { const value = patchPath.pop(); const edit = modify(configString, patchPath, value, { isArrayInsertion: true, }); configString = applyEdits(configString, edit); - const formatEdit = format(configString, undefined, {}); - configString = applyEdits(configString, formatEdit); } + const formatEdit = format(configString, undefined, {}); + configString = applyEdits(configString, formatEdit); if (configPath.endsWith(".toml")) { - return TOML.stringify(JSONCParse(configString)); + configString = TOML.stringify(parseJSONC(configString)); } + writeFileSync(configPath, configString); return configString; }; -const getPath = ( +// gets all the json paths for the patch which are needed to create the edit +const getJSONPath = ( obj: RawConfig, allPaths: JSONPath[], prevPath: JSONPath = [] @@ -52,12 +53,20 @@ const getPath = ( const currentPath = [...prevPath, k]; if (Array.isArray(v)) { v.forEach((x) => { + // makes sure we insert new array items at the end + // currently this function is additive, ie it assumes a patch with an array item should be added, + // rather than replacing (modifying, deleting) an existing item at the index allPaths.push([...currentPath, -1, x]); }); } else if (typeof v === "object") { - getPath(v, allPaths, currentPath); + getJSONPath(v, allPaths, currentPath); } else { allPaths.push([...currentPath, v]); } } }; + +/** + * Custom error class for config patching errors + */ +export class PatchConfigError extends Error {} From 4fa336bdefb42318340a9f597d94536c3dbf9f84 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 16 Dec 2024 18:53:57 +0000 Subject: [PATCH 09/13] allow editing and deleting with `isArrayInsertion` flag --- .../src/__tests__/patch-config.test.ts | 514 +++++++++++++++--- packages/wrangler/src/config/patch-config.ts | 45 +- 2 files changed, 473 insertions(+), 86 deletions(-) diff --git a/packages/wrangler/src/__tests__/patch-config.test.ts b/packages/wrangler/src/__tests__/patch-config.test.ts index 258ba0ce1017..c1f43a60396c 100644 --- a/packages/wrangler/src/__tests__/patch-config.test.ts +++ b/packages/wrangler/src/__tests__/patch-config.test.ts @@ -8,7 +8,8 @@ import type { RawConfig } from "../config"; type TestCase = { name: string; original: RawConfig; - patch: RawConfig; + additivePatch: RawConfig; + replacingPatch: RawConfig; expectedToml: string; expectedJson: string; }; @@ -16,7 +17,14 @@ const testCases: TestCase[] = [ { name: "add a binding", original: {}, - patch: { + additivePatch: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + replacingPatch: { kv_namespaces: [ { binding: "KV", @@ -52,8 +60,18 @@ const testCases: TestCase[] = [ }, ], }, - patch: { + additivePatch: { + kv_namespaces: [ + { + binding: "KV2", + }, + ], + }, + replacingPatch: { kv_namespaces: [ + { + binding: "KV", + }, { binding: "KV2", }, @@ -94,7 +112,14 @@ const testCases: TestCase[] = [ }, ], }, - patch: { + additivePatch: { + d1_databases: [ + { + binding: "DB", + }, + ], + }, + replacingPatch: { d1_databases: [ { binding: "DB", @@ -138,7 +163,10 @@ const testCases: TestCase[] = [ }, ], }, - patch: { + additivePatch: { + compatibility_flags: ["nodejs_compat"], + }, + replacingPatch: { compatibility_flags: ["nodejs_compat"], }, expectedToml: dedent` @@ -179,8 +207,26 @@ const testCases: TestCase[] = [ }, ], }, - patch: { + additivePatch: { + kv_namespaces: [ + { + binding: "KV2", + }, + { + binding: "KV3", + }, + ], + d1_databases: [ + { + binding: "DB2", + }, + ], + }, + replacingPatch: { kv_namespaces: [ + { + binding: "KV", + }, { binding: "KV2", }, @@ -189,6 +235,9 @@ const testCases: TestCase[] = [ }, ], d1_databases: [ + { + binding: "DB", + }, { binding: "DB2", }, @@ -242,99 +291,412 @@ const testCases: TestCase[] = [ }, ]; +const replacingOnlyTestCases: Omit[] = [ + { + name: "edit a binding", + original: {}, + replacingPatch: { + kv_namespaces: [ + { + binding: "KV2", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV2" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV2" + } + ] + } + `, + }, + { + name: "add a field to an existing binding", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + replacingPatch: { + kv_namespaces: [ + { + binding: "KV", + id: "1234", + }, + ], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + [[kv_namespaces]] + binding = "KV" + id = "1234" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + "binding": "KV", + "id": "1234" + } + ] + } + `, + }, + { + name: "edit a compat flag", + original: {}, + replacingPatch: { + compatibility_flags: ["no_nodejs_compat"], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + compatibility_flags = [ "no_nodejs_compat" ] + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "compatibility_flags": [ + "no_nodejs_compat" + ] + } + `, + }, + { + name: "add a compat flag", + original: { compatibility_flags: ["nodejs_compat"] }, + replacingPatch: { + compatibility_flags: ["nodejs_compat", "flag"], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + compatibility_flags = [ "nodejs_compat", "flag" ] + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name", + "compatibility_flags": [ + "nodejs_compat", + "flag" + ] + } + `, + }, + { + name: "delete a compat flag", + original: {}, + replacingPatch: { + compatibility_flags: [], + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name" + } + `, + }, +]; + describe("experimental_patchConfig()", () => { runInTempDir(); - describe.each(["json", "toml"])("%s", (configType) => { - it.each(testCases)( + describe.each([true, false])("isArrayInsertion = %o", (isArrayInsertion) => { + describe.each(testCases)( `$name`, - ({ original, patch, expectedJson, expectedToml }) => { - writeWranglerConfig( - original, - configType === "json" ? "./wrangler.json" : "./wrangler.toml" - ); - const result = experimental_patchConfig( - configType === "json" ? "./wrangler.json" : "./wrangler.toml", - patch - ); - expect(result).not.toBeFalsy(); - expect(result).toEqual( - `${configType === "json" ? expectedJson : expectedToml}` - ); + ({ + original, + replacingPatch, + additivePatch, + expectedJson, + expectedToml, + }) => { + it.each(["json", "toml"])("%s", (configType) => { + writeWranglerConfig( + original, + configType === "json" ? "./wrangler.json" : "./wrangler.toml" + ); + const result = experimental_patchConfig( + configType === "json" ? "./wrangler.json" : "./wrangler.toml", + isArrayInsertion ? additivePatch : replacingPatch, + isArrayInsertion + ); + expect(result).not.toBeFalsy(); + expect(result).toEqual( + `${configType === "json" ? expectedJson : expectedToml}` + ); + }); + } + ); + }); + describe("isArrayInsertion = false", () => { + describe.each(replacingOnlyTestCases)( + `$name`, + ({ original, replacingPatch, expectedJson, expectedToml }) => { + it.each(["json", "toml"])("%s", (configType) => { + writeWranglerConfig( + original, + configType === "json" ? "./wrangler.json" : "./wrangler.toml" + ); + const result = experimental_patchConfig( + configType === "json" ? "./wrangler.json" : "./wrangler.toml", + replacingPatch, + false + ); + expect(result).not.toBeFalsy(); + expect(result).toEqual( + `${configType === "json" ? expectedJson : expectedToml}` + ); + }); } ); }); describe("jsonc", () => { - it("preserves comments", () => { - const jsonc = ` - { - // a comment - "compatibility_date": "2022-01-12", - "name": "test-name", - "kv_namespaces": [ - { - // more comments! - "binding": "KV" - } - ], - "d1_databases": [ - /** - * multiline comment - */ - { - "binding": "DB" - } - ] - } - `; - writeFileSync("./wrangler.jsonc", jsonc); - const patch = { - kv_namespaces: [ - { - binding: "KV2", - }, - { - binding: "KV3", - }, - ], - d1_databases: [ - { - binding: "DB2", - }, - ], - }; - const result = experimental_patchConfig("./wrangler.jsonc", patch); - expect(result).not.toBeFalsy(); - expect(result).toMatchInlineSnapshot(` - "{ + describe("add multiple bindings", () => { + it("isArrayInsertion = true", () => { + const jsonc = ` + { // a comment - \\"compatibility_date\\": \\"2022-01-12\\", - \\"name\\": \\"test-name\\", - \\"kv_namespaces\\": [ + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ { // more comments! - \\"binding\\": \\"KV\\" + "binding": "KV" + } + ], + "d1_databases": [ + /** + * multiline comment + */ + { + "binding": "DB" + } + ] + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const patch = { + kv_namespaces: [ + { + binding: "KV2", }, { - \\"binding\\": \\"KV2\\" + binding: "KV3", }, + ], + d1_databases: [ { - \\"binding\\": \\"KV3\\" - } + binding: "DB2", + }, ], - \\"d1_databases\\": [ - /** + }; + const result = experimental_patchConfig("./wrangler.jsonc", patch); + expect(result).not.toBeFalsy(); + expect(result).toMatchInlineSnapshot(` + "{ + // a comment + \\"compatibility_date\\": \\"2022-01-12\\", + \\"name\\": \\"test-name\\", + \\"kv_namespaces\\": [ + { + // more comments! + \\"binding\\": \\"KV\\" + }, + { + \\"binding\\": \\"KV2\\" + }, + { + \\"binding\\": \\"KV3\\" + } + ], + \\"d1_databases\\": [ + /** + * multiline comment + */ + { + \\"binding\\": \\"DB\\" + }, + { + \\"binding\\": \\"DB2\\" + } + ] + }" + `); + }); + it("isArrayInsertion = false ", () => { + const jsonc = dedent` + { + // a comment + "compatibility_date": "2022-01-12", + "name": "test-name", + "kv_namespaces": [ + { + // more comments! + "binding": "KV" + } + ], + "d1_databases": [ + /** + * multiline comment + */ + { + "binding": "DB" + } + ] + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const patch = { + kv_namespaces: [ + { + binding: "KV", + }, + { + binding: "KV2", + }, + { + binding: "KV3", + }, + ], + d1_databases: [ + { + binding: "DB", + }, + { + binding: "DB2", + }, + ], + }; + const result = experimental_patchConfig( + "./wrangler.jsonc", + patch, + false + ); + expect(result).not.toBeFalsy(); + expect(result).toMatchInlineSnapshot(` + "{ + // a comment + \\"compatibility_date\\": \\"2022-01-12\\", + \\"name\\": \\"test-name\\", + \\"kv_namespaces\\": [ + { + // more comments! + \\"binding\\": \\"KV\\" + }, + { + \\"binding\\": \\"KV2\\" + }, + { + \\"binding\\": \\"KV3\\" + } + ], + \\"d1_databases\\": [ + /** * multiline comment */ + { + \\"binding\\": \\"DB\\" + }, + { + \\"binding\\": \\"DB2\\" + } + ] + }" + `); + }); + }); + describe("edit existing bindings", () => { + it("isArrayInsertion = false", () => { + const jsonc = ` + { + // comment one + "compatibility_date": "2022-01-12", + // comment two + "name": "test-name", + "kv_namespaces": [ { - \\"binding\\": \\"DB\\" + // comment three + "binding": "KV" + // comment four }, { - \\"binding\\": \\"DB2\\" + // comment five + "binding": "KV2" + // comment six } ] - }" - `); + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const patch = { + compatibility_date: "2024-27-09", + kv_namespaces: [ + { + binding: "KV", + id: "hello-id", + }, + { + binding: "KV2", + }, + ], + }; + const result = experimental_patchConfig( + "./wrangler.jsonc", + patch, + false + ); + expect(result).not.toBeFalsy(); + expect(result).toMatchInlineSnapshot(` + "{ + // comment one + \\"compatibility_date\\": \\"2024-27-09\\", + // comment two + \\"name\\": \\"test-name\\", + \\"kv_namespaces\\": [ + { + // comment three + \\"binding\\": \\"KV\\", + \\"id\\": \\"hello-id\\" + // comment four + }, + { + // comment five + \\"binding\\": \\"KV2\\" + // comment six + } + ] + }" + `); + }); }); }); }); diff --git a/packages/wrangler/src/config/patch-config.ts b/packages/wrangler/src/config/patch-config.ts index d28128b7c7c6..38b09ce2b4e3 100644 --- a/packages/wrangler/src/config/patch-config.ts +++ b/packages/wrangler/src/config/patch-config.ts @@ -7,7 +7,16 @@ import type { JSONPath } from "jsonc-parser"; export const experimental_patchConfig = ( configPath: string, - patch: RawConfig + /** + * if you want to add something new, e.g. a binding, you can just provide that {kv_namespace:[{binding:"KV"}]} + * and set isArrayInsertion = true + * + * if you want to edit or delete existing array elements, you have to provide the whole array + * e.g. {kv_namespace:[{binding:"KV", id:"new-id"}, {binding:"KV2", id:"untouched"}]} + * and set isArrayInsertion = false + */ + patch: RawConfig, + isArrayInsertion: boolean = true ) => { let configString = readFileSync(configPath); @@ -25,11 +34,11 @@ export const experimental_patchConfig = ( } const patchPaths: JSONPath[] = []; - getJSONPath(patch, patchPaths); + getJSONPath(patch, patchPaths, isArrayInsertion); for (const patchPath of patchPaths) { const value = patchPath.pop(); const edit = modify(configString, patchPath, value, { - isArrayInsertion: true, + isArrayInsertion, }); configString = applyEdits(configString, edit); } @@ -43,23 +52,39 @@ export const experimental_patchConfig = ( return configString; }; -// gets all the json paths for the patch which are needed to create the edit +/** + * + * Gets all the JSON paths for a given object by recursing through the object, recording the properties encountered. + * e.g. {a : { b: "c", d: ["e", "f"]}} -> [["a", "b", "c"], ["a", "d", 0], ["a", "d", 1]] + * The jsonc-parser library requires JSON paths for each edit. + * Note the final 'path' segment is the value we want to insert, + * so in the above example,["a", "b"] would be the path and we would insert "c" + * + * If isArrayInsertion = false, when we encounter an array, we use the item index as part of the path and continue + * If isArrayInsertion = false, we stop recursing down and treat the whole array item as the final path segment/value. + * + */ const getJSONPath = ( obj: RawConfig, allPaths: JSONPath[], + isArrayInsertion: boolean, prevPath: JSONPath = [] ) => { for (const [k, v] of Object.entries(obj)) { const currentPath = [...prevPath, k]; if (Array.isArray(v)) { - v.forEach((x) => { - // makes sure we insert new array items at the end - // currently this function is additive, ie it assumes a patch with an array item should be added, - // rather than replacing (modifying, deleting) an existing item at the index - allPaths.push([...currentPath, -1, x]); + v.forEach((x, i) => { + if (isArrayInsertion) { + // makes sure we insert new array items at the end + allPaths.push([...currentPath, -1, x]); + } else if (typeof x === "object") { + getJSONPath(x, allPaths, isArrayInsertion, [...currentPath, i]); + } else { + allPaths.push([...currentPath, i, x]); + } }); } else if (typeof v === "object") { - getJSONPath(v, allPaths, currentPath); + getJSONPath(v, allPaths, isArrayInsertion, currentPath); } else { allPaths.push([...currentPath, v]); } From 577e579d99b63b7f46174d35c4482b2377ef96ba Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 17 Dec 2024 12:03:39 +0000 Subject: [PATCH 10/13] more tests --- .../src/__tests__/patch-config.test.ts | 180 +++++++++++++++++- 1 file changed, 179 insertions(+), 1 deletion(-) diff --git a/packages/wrangler/src/__tests__/patch-config.test.ts b/packages/wrangler/src/__tests__/patch-config.test.ts index c1f43a60396c..91972e604f58 100644 --- a/packages/wrangler/src/__tests__/patch-config.test.ts +++ b/packages/wrangler/src/__tests__/patch-config.test.ts @@ -361,6 +361,73 @@ const replacingOnlyTestCases: Omit[] = [ } `, }, + { + name: "delete an existing binding so that none are left", + original: { + kv_namespaces: [ + { + binding: "KV", + }, + ], + }, + replacingPatch: { + kv_namespaces: undefined, + }, + expectedToml: dedent` + compatibility_date = "2022-01-12" + name = "test-name" + + `, + expectedJson: dedent` + { + "compatibility_date": "2022-01-12", + "name": "test-name" + } + `, + }, + // This one doesn't work because the jsonc-parser leaves behind a stray bracket when deleting + // there are possibly solutions that I am not inclined to solve right now + // e.g. passing in {binding: undefined} in the patch instead and cleaning up empty objects + // { + // name: "delete an existing binding (but some bindings of that type are still left)", + // original: { + // kv_namespaces: [ + // { + // binding: "KV", + // }, + // { + // binding: "KV2", + // }, + // ], + // }, + // replacingPatch: { + // kv_namespaces: [ + // { + // binding: "KV", + // }, + // undefined, + // ], + // }, + // expectedToml: dedent` + // compatibility_date = "2022-01-12" + // name = "test-name" + + // [[kv_namespaces]] + // binding = "KV" + + // `, + // expectedJson: dedent` + // { + // "compatibility_date": "2022-01-12", + // "name": "test-name", + // "kv_namespaces": [ + // { + // "binding": "KV" + // } + // ] + // } + // `, + // }, { name: "edit a compat flag", original: {}, @@ -410,7 +477,7 @@ const replacingOnlyTestCases: Omit[] = [ name: "delete a compat flag", original: {}, replacingPatch: { - compatibility_flags: [], + compatibility_flags: undefined, }, expectedToml: dedent` compatibility_date = "2022-01-12" @@ -698,5 +765,116 @@ describe("experimental_patchConfig()", () => { `); }); }); + + describe("edit existing bindings with patch array in a different order (will mess up comments)", () => { + it("isArrayInsertion = false", () => { + const jsonc = ` + { + // comment one + "compatibility_date": "2022-01-12", + // comment two + "name": "test-name", + "kv_namespaces": [ + { + // comment three + "binding": "KV" + // comment four + }, + { + // comment five + "binding": "KV2" + // comment six + } + ] + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const patch = { + compatibility_date: "2024-27-09", + kv_namespaces: [ + { + binding: "KV2", + }, + { + binding: "KV", + id: "hello-id", + }, + ], + }; + const result = experimental_patchConfig( + "./wrangler.jsonc", + patch, + false + ); + expect(result).not.toBeFalsy(); + // Note that the comments have stayed in place! + // However, I don't think we can reasonably expect to bring comments along when an array has been reordered + expect(result).toMatchInlineSnapshot(` + "{ + // comment one + \\"compatibility_date\\": \\"2024-27-09\\", + // comment two + \\"name\\": \\"test-name\\", + \\"kv_namespaces\\": [ + { + // comment three + \\"binding\\": \\"KV2\\" + // comment four + }, + { + // comment five + \\"binding\\": \\"KV\\", + \\"id\\": \\"hello-id\\" + // comment six + } + ] + }" + `); + }); + }); + + describe("delete existing bindings (cannot preserve comments)", () => { + it("isArrayInsertion = false", () => { + const jsonc = ` + { + // comment one + "compatibility_date": "2022-01-12", + // comment two + "name": "test-name", + "kv_namespaces": [ + { + // comment three + "binding": "KV" + // comment four + }, + { + // comment five + "binding": "KV2" + // comment six + } + ] + } + `; + writeFileSync("./wrangler.jsonc", jsonc); + const patch = { + compatibility_date: "2024-27-09", + kv_namespaces: undefined, + }; + const result = experimental_patchConfig( + "./wrangler.jsonc", + patch, + false + ); + expect(result).not.toBeFalsy(); + expect(result).toMatchInlineSnapshot(` + "{ + // comment one + \\"compatibility_date\\": \\"2024-27-09\\", + // comment two + \\"name\\": \\"test-name\\" + }" + `); + }); + }); }); }); From 80b5ac99a8a1ed194d51d29ed000e029baab012d Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:32:35 +0000 Subject: [PATCH 11/13] fixups --- .../src/__tests__/configuration.test.ts | 32 +++++++++++-------- packages/wrangler/src/cli.ts | 1 + 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts index 01d2b05185ef..5083e0a84602 100644 --- a/packages/wrangler/src/__tests__/configuration.test.ts +++ b/packages/wrangler/src/__tests__/configuration.test.ts @@ -6045,15 +6045,19 @@ describe("experimental_readRawConfig()", () => { runInTempDir(); it(`should find a ${configType} config file given a specific path`, () => { fs.mkdirSync("../folder", { recursive: true }); - writeWranglerConfig({}, `../folder/config.${configType}`); + writeWranglerConfig( + { name: "config-one" }, + `../folder/config.${configType}` + ); const result = experimental_readRawConfig({ config: `../folder/config.${configType}`, }); - expect(result.rawConfig).toEqual({ - compatibility_date: "2022-01-12", - name: "test-name", - }); + expect(result.rawConfig).toEqual( + expect.objectContaining({ + name: "config-one", + }) + ); }); it("should find a config file given a specific script", () => { @@ -6072,18 +6076,20 @@ describe("experimental_readRawConfig()", () => { let result = experimental_readRawConfig({ script: "./path/to/index.js", }); - expect(result.rawConfig).toEqual({ - compatibility_date: "2022-01-12", - name: "config-one", - }); + expect(result.rawConfig).toEqual( + expect.objectContaining({ + name: "config-one", + }) + ); result = experimental_readRawConfig({ script: "../folder/index.js", }); - expect(result.rawConfig).toEqual({ - compatibility_date: "2022-01-12", - name: "config-two", - }); + expect(result.rawConfig).toEqual( + expect.objectContaining({ + name: "config-two", + }) + ); }); } ); diff --git a/packages/wrangler/src/cli.ts b/packages/wrangler/src/cli.ts index 6da850958a54..f3139526b21f 100644 --- a/packages/wrangler/src/cli.ts +++ b/packages/wrangler/src/cli.ts @@ -60,3 +60,4 @@ const generateASSETSBinding: ( export { generateASSETSBinding as unstable_generateASSETSBinding }; export { experimental_readRawConfig } from "./config"; +export { experimental_patchConfig } from "./config/patch-config"; From 4675496fbec19cb2701baa612ba0db6a383120a1 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:36:28 +0000 Subject: [PATCH 12/13] update changeset --- .changeset/lovely-rats-live.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.changeset/lovely-rats-live.md b/.changeset/lovely-rats-live.md index fb1a8a11e4e1..2b08b4e9ed48 100644 --- a/.changeset/lovely-rats-live.md +++ b/.changeset/lovely-rats-live.md @@ -2,9 +2,6 @@ "wrangler": patch --- -feat: add experimental_patchConfig() and experimental_readRawConfig() +feat: add experimental_patchConfig() -Adds two Wrangler APIs: - -1. experimental_readRawConfig() will find and read a config file -2. experimental_patchConfig() can add to a user's config file. It preserves comments if its a `wrangler.jsonc`. However, it is not suitable for `wrangler.toml` with comments as we cannot preserve comments on write. +`experimental_patchConfig()` can add to a user's config file. It preserves comments if its a `wrangler.jsonc`. However, it is not suitable for `wrangler.toml` with comments as we cannot preserve comments on write. From 6c2ce25fd009b90bb124783d827bedb069d12fe9 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 18 Dec 2024 16:54:57 +0000 Subject: [PATCH 13/13] fix test --- packages/wrangler/src/__tests__/patch-config.test.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/wrangler/src/__tests__/patch-config.test.ts b/packages/wrangler/src/__tests__/patch-config.test.ts index 91972e604f58..1f12784883f7 100644 --- a/packages/wrangler/src/__tests__/patch-config.test.ts +++ b/packages/wrangler/src/__tests__/patch-config.test.ts @@ -364,14 +364,10 @@ const replacingOnlyTestCases: Omit[] = [ { name: "delete an existing binding so that none are left", original: { - kv_namespaces: [ - { - binding: "KV", - }, - ], + compatibility_flags: ["test-flag"], }, replacingPatch: { - kv_namespaces: undefined, + compatibility_flags: undefined, }, expectedToml: dedent` compatibility_date = "2022-01-12"