From 3a26b0c04a5c69e7f0b22b781812c7cad876eb0a Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Tue, 28 Nov 2023 13:09:14 +0100 Subject: [PATCH 1/9] feat: Support jsonc config files --- deno.lock | 1 + deployctl.ts | 4 +- deps.ts | 3 ++ src/config_file.ts | 119 ++++++++++++++++++++++++++++++++++++++------- 4 files changed, 108 insertions(+), 19 deletions(-) diff --git a/deno.lock b/deno.lock index dbbd1fe4..0d6f0f57 100644 --- a/deno.lock +++ b/deno.lock @@ -17,6 +17,7 @@ "https://deno.land/std@0.116.0/testing/asserts.ts": "a1fef0239a2c343b0baa49c77dcdd7412613c46f3aba2887c331a2d7ed1f645e", "https://deno.land/std@0.170.0/_util/asserts.ts": "d0844e9b62510f89ce1f9878b046f6a57bf88f208a10304aab50efcb48365272", "https://deno.land/std@0.170.0/_util/os.ts": "8a33345f74990e627b9dfe2de9b040004b08ea5146c7c9e8fe9a29070d193934", + "https://deno.land/std@0.170.0/encoding/jsonc.ts": "79289a5a8c0d9f82b536dfa1868c135d325791ecb2a38740f635eac17fef554f", "https://deno.land/std@0.170.0/fmt/colors.ts": "03ad95e543d2808bc43c17a3dd29d25b43d0f16287fe562a0be89bf632454a12", "https://deno.land/std@0.170.0/path/_constants.ts": "df1db3ffa6dd6d1252cc9617e5d72165cd2483df90e93833e13580687b6083c3", "https://deno.land/std@0.170.0/path/_interface.ts": "ee3b431a336b80cf445441109d089b70d87d5e248f4f90ff906820889ecf8d09", diff --git a/deployctl.ts b/deployctl.ts index cef5b1ca..3dacb71e 100755 --- a/deployctl.ts +++ b/deployctl.ts @@ -114,7 +114,9 @@ async function setDefaultsFromConfigFile(args: Args) { args.config ?? configFile.cwdOrAncestors(), ); if (config === null && args.config !== undefined && !args["save-config"]) { - error(`Could not find or read the config file '${args.config}'`); + error( + `Could not find or read the config file '${args.config}'. Use --save-config to create it.`, + ); } if (config !== null) { wait("").start().info(`Using config file '${config.path()}'`); diff --git a/deps.ts b/deps.ts index 978b9537..48b5aac1 100644 --- a/deps.ts +++ b/deps.ts @@ -4,6 +4,7 @@ export { basename, dirname, + extname, fromFileUrl, join, normalize, @@ -13,6 +14,7 @@ export { } from "https://deno.land/std@0.170.0/path/mod.ts"; export { bold, + cyan, green, magenta, red, @@ -20,6 +22,7 @@ export { } from "https://deno.land/std@0.170.0/fmt/colors.ts"; export { parse } from "https://deno.land/std@0.195.0/flags/mod.ts"; export { TextLineStream } from "https://deno.land/std@0.170.0/streams/text_line_stream.ts"; +export * as JSONC from "https://deno.land/std@0.170.0/encoding/jsonc.ts"; // x/semver export { diff --git a/src/config_file.ts b/src/config_file.ts index 6e74741f..a7152d28 100644 --- a/src/config_file.ts +++ b/src/config_file.ts @@ -1,11 +1,23 @@ // Copyright 2021 Deno Land Inc. All rights reserved. MIT license. -import { dirname, join, relative, resolve } from "../deps.ts"; +import { + cyan, + dirname, + extname, + green, + join, + JSONC, + magenta, + red, + relative, + resolve, +} from "../deps.ts"; import { error } from "./error.ts"; import { isURL } from "./utils/entrypoint.ts"; import { wait } from "./utils/spinner.ts"; const DEFAULT_FILENAME = "deno.json"; +const CANDIDATE_FILENAMES = [DEFAULT_FILENAME, "deno.jsonc"]; /** Arguments persisted in the deno.json config file */ interface ConfigArgs { @@ -61,20 +73,26 @@ class ConfigFile { } } - /** Check if the `ConfigArgs` in this `ConfigFile` match `args` + /** Returns all the differences between this `ConfigArgs` and the one provided as argument. * - * Ignores any property in `args` not meant to be persisted. + * The comparison is performed against the JSON output of each config. The "other" args are + * sematically considered additions in the return value. Ignores any property in `args` not meant + * to be persisted. */ - eq(args: ConfigArgs) { - const otherConfigArgs = this.normalize(args); + diff(args: ConfigArgs): Change[] { + const changes = []; + const otherConfigOutput = + JSON.parse(ConfigFile.create(this.path(), args).toFileContent()).deploy ?? + {}; + const thisConfigOutput = JSON.parse(this.toFileContent()).deploy ?? {}; // Iterate over the other args as they might include args not yet persisted in the config file - for (const [key, otherValue] of Object.entries(otherConfigArgs)) { - // deno-lint-ignore no-explicit-any - if ((this.args() as any)[key] !== otherValue) { - return false; + for (const [key, otherValue] of Object.entries(otherConfigOutput)) { + const thisValue = thisConfigOutput[key]; + if (thisValue !== otherValue) { + changes.push({ key, removal: thisValue, addition: otherValue }); } } - return true; + return changes; } normalize(args: ConfigArgs): ConfigArgs { @@ -95,7 +113,9 @@ class ConfigFile { } static fromFileContent(filepath: string, content: string) { - const parsedContent = JSON.parse(content); + const parsedContent = extname(filepath) === ".jsonc" + ? new Object(JSONC.parse(content)) + : JSON.parse(content); const configContent = { ...parsedContent, deploy: parsedContent.deploy && { @@ -175,19 +195,49 @@ export default { overwrite: boolean, ): Promise { const pathOrDefault = path ?? DEFAULT_FILENAME; + const isJsonc = extname(pathOrDefault) === ".jsonc"; const existingConfig = await this.read(pathOrDefault); + const changes = existingConfig?.diff(args); let config; - if (existingConfig && existingConfig.hasDeployConfig() && !overwrite) { - if (!existingConfig.eq(args)) { - wait("").start().info( - `Some of the config used differ from the config found in '${existingConfig.path()}'. Use --save-config to overwrite it.`, - ); - } + if (existingConfig && changes!.length === 0) { + // There are no changes to write + return; + } else if ( + existingConfig && existingConfig.hasDeployConfig() && !overwrite + ) { + // There are changes to write and there's already some deploy config, we require the --save-config flag + wait("").start().info( + `Some of the config used differ from the config found in '${existingConfig.path()}'. Use --save-config to overwrite it.`, + ); return; } else if (existingConfig) { + // Either there is no deploy config in the config file or the user is using --save-config flag + if (isJsonc) { + const msg = overwrite + ? `Writing to the config file '${pathOrDefault}' will remove any existing comment and format it as a plain JSON file. Is that ok?` + : `I want to store some configuration in '${pathOrDefault}' config file but this will remove any existing comment from it. Is that ok?`; + const confirmation = confirm(`${magenta("?")} ${msg}`); + if (!confirmation) { + const formattedChanges = existingConfig.hasDeployConfig() + ? cyan( + ` "deploy": {\n ...\n${formatChanges(changes!, 2, 2)}\n }`, + ) + : green( + ConfigFile.create(pathOrDefault, args).toFileContent().slice( + 2, + -2, + ), + ); + wait({ text: "", indent: 3 }).start().info( + `I understand. Here's the config I wanted to write:\n${formattedChanges}`, + ); + return; + } + } existingConfig.override(args); config = existingConfig; } else { + // The config file does not exist. Create a new one. config = ConfigFile.create(pathOrDefault, args); } await Deno.writeTextFile( @@ -204,7 +254,9 @@ export default { cwdOrAncestors: function* () { let wd = Deno.cwd(); while (wd) { - yield join(wd, DEFAULT_FILENAME); + for (const filename of CANDIDATE_FILENAMES) { + yield join(wd, filename); + } const newWd = dirname(wd); if (newWd === wd) { return; @@ -214,3 +266,34 @@ export default { } }, }; + +function formatChanges( + changes: Change[], + indent?: number, + gap?: number, +): string { + const removals = []; + const additions = []; + const padding = " ".repeat(indent ?? 0); + const innerPadding = " ".repeat(gap ?? 0); + for (const { key, removal, addition } of changes) { + if (removal !== undefined) { + removals.push(red( + `${padding}-${innerPadding}"${key}": ${JSON.stringify(removal)}`, + )); + } + if (addition !== undefined) { + additions.push(green( + `${padding}+${innerPadding}"${key}": ${JSON.stringify(addition)}`, + )); + } + } + return [removals.join(red(",\n")), additions.join(green(",\n"))].join("\n") + .trim(); +} + +interface Change { + key: string; + removal?: unknown; + addition?: unknown; +} From 8477f4de6fc5c5eaed7a1237cb04fcbe1609bc1a Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Thu, 30 Nov 2023 11:39:36 +0100 Subject: [PATCH 2/9] Apply suggestion --- src/config_file.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config_file.ts b/src/config_file.ts index a7152d28..61b40629 100644 --- a/src/config_file.ts +++ b/src/config_file.ts @@ -197,9 +197,9 @@ export default { const pathOrDefault = path ?? DEFAULT_FILENAME; const isJsonc = extname(pathOrDefault) === ".jsonc"; const existingConfig = await this.read(pathOrDefault); - const changes = existingConfig?.diff(args); + const changes = existingConfig?.diff(args) ?? []; let config; - if (existingConfig && changes!.length === 0) { + if (existingConfig && changes.length === 0) { // There are no changes to write return; } else if ( @@ -220,7 +220,7 @@ export default { if (!confirmation) { const formattedChanges = existingConfig.hasDeployConfig() ? cyan( - ` "deploy": {\n ...\n${formatChanges(changes!, 2, 2)}\n }`, + ` "deploy": {\n ...\n${formatChanges(changes, 2, 2)}\n }`, ) : green( ConfigFile.create(pathOrDefault, args).toFileContent().slice( From fa022899b1d72f1a61e24222c3b7c85c74cb2713 Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Thu, 30 Nov 2023 12:06:50 +0100 Subject: [PATCH 3/9] Always juse JSONC.parse --- src/config_file.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/config_file.ts b/src/config_file.ts index 61b40629..4c6a105e 100644 --- a/src/config_file.ts +++ b/src/config_file.ts @@ -113,9 +113,7 @@ class ConfigFile { } static fromFileContent(filepath: string, content: string) { - const parsedContent = extname(filepath) === ".jsonc" - ? new Object(JSONC.parse(content)) - : JSON.parse(content); + const parsedContent = JSONC.parse(content) as { deploy?: ConfigArgs }; const configContent = { ...parsedContent, deploy: parsedContent.deploy && { From 1711810957c0936315000722afc8c4285dbea64e Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Thu, 30 Nov 2023 12:51:05 +0100 Subject: [PATCH 4/9] Add unit test --- tests/config_file_test/config.json | 1 + tests/config_file_test/config_file_test.ts | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/config_file_test/config.json create mode 100644 tests/config_file_test/config_file_test.ts diff --git a/tests/config_file_test/config.json b/tests/config_file_test/config.json new file mode 100644 index 00000000..9e26dfee --- /dev/null +++ b/tests/config_file_test/config.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/tests/config_file_test/config_file_test.ts b/tests/config_file_test/config_file_test.ts new file mode 100644 index 00000000..a6cf91a0 --- /dev/null +++ b/tests/config_file_test/config_file_test.ts @@ -0,0 +1,22 @@ +import configFile from "../../src/config_file.ts"; +import { assert, assertEquals } from "../deps.ts"; + +Deno.test("ConfigFile.diff returns array with additions and removals", async () => { + const config = await configFile.read(new URL(import.meta.resolve("./config.json")).pathname); + assert(!!config); + + let changes = config.diff({}); + assertEquals(changes, []); + + changes = config.diff({ project: "foo" }); + assertEquals(changes, [{ key: "project", addition: "foo", removal: undefined }]); + + // Using file URLs to avoid dealing with path normalization + config.override({ project: "foo", entrypoint: "file://main.ts" }); + + changes = config.diff({ project: "bar", entrypoint: "file://src/main.ts" }); + assertEquals(changes, [ + { key: "project", removal: "foo", addition: "bar" }, + { key: "entrypoint", removal: "file://main.ts", addition: "file://src/main.ts" }, + ]); +}); From 8659a923c6ada28959947224a619d75ef3e566af Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Thu, 30 Nov 2023 14:31:31 +0100 Subject: [PATCH 5/9] fixes --- src/config_file.ts | 7 +++---- tests/config_file_test/config_file_test.ts | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/config_file.ts b/src/config_file.ts index 263cd9d8..57e98253 100644 --- a/src/config_file.ts +++ b/src/config_file.ts @@ -90,11 +90,10 @@ class ConfigFile { // Iterate over the other args as they might include args not yet persisted in the config file for (const [key, otherValue] of Object.entries(otherConfigOutput)) { const thisValue = thisConfigOutput[key]; - if (otherValue instanceof Array) { - const thisArrayValue = thisValue as typeof otherValue; + if (otherValue instanceof Array && thisValue instanceof Array) { if ( - thisArrayValue.length !== otherValue.length || - !thisArrayValue.every((x, i) => otherValue[i] === x) + thisValue.length !== otherValue.length || + !thisValue.every((x, i) => otherValue[i] === x) ) { changes.push({ key, removal: thisValue, addition: otherValue }); } diff --git a/tests/config_file_test/config_file_test.ts b/tests/config_file_test/config_file_test.ts index a6cf91a0..a3dc8de0 100644 --- a/tests/config_file_test/config_file_test.ts +++ b/tests/config_file_test/config_file_test.ts @@ -1,22 +1,33 @@ +import { fromFileUrl } from "../../deps.ts"; import configFile from "../../src/config_file.ts"; import { assert, assertEquals } from "../deps.ts"; Deno.test("ConfigFile.diff returns array with additions and removals", async () => { - const config = await configFile.read(new URL(import.meta.resolve("./config.json")).pathname); + const config = await configFile.read( + fromFileUrl(new URL(import.meta.resolve("./config.json"))), + ); assert(!!config); let changes = config.diff({}); assertEquals(changes, []); changes = config.diff({ project: "foo" }); - assertEquals(changes, [{ key: "project", addition: "foo", removal: undefined }]); + assertEquals(changes, [{ + key: "project", + addition: "foo", + removal: undefined, + }]); - // Using file URLs to avoid dealing with path normalization + // Using file URLs to avoid dealing with path normalization config.override({ project: "foo", entrypoint: "file://main.ts" }); changes = config.diff({ project: "bar", entrypoint: "file://src/main.ts" }); assertEquals(changes, [ { key: "project", removal: "foo", addition: "bar" }, - { key: "entrypoint", removal: "file://main.ts", addition: "file://src/main.ts" }, + { + key: "entrypoint", + removal: "file://main.ts", + addition: "file://src/main.ts", + }, ]); }); From 293b39120712d009d9879136a3d3e225f4874f50 Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Thu, 30 Nov 2023 15:05:51 +0100 Subject: [PATCH 6/9] fmt --- tests/config_file_test/config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config_file_test/config.json b/tests/config_file_test/config.json index 9e26dfee..0967ef42 100644 --- a/tests/config_file_test/config.json +++ b/tests/config_file_test/config.json @@ -1 +1 @@ -{} \ No newline at end of file +{} From 437f373f252eb28cbaf4bfa36bc3a79456a06c02 Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Fri, 1 Dec 2023 13:27:27 +0100 Subject: [PATCH 7/9] add test --- tests/config_file_test/config_file_test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/config_file_test/config_file_test.ts b/tests/config_file_test/config_file_test.ts index a3dc8de0..3ab3473b 100644 --- a/tests/config_file_test/config_file_test.ts +++ b/tests/config_file_test/config_file_test.ts @@ -31,3 +31,21 @@ Deno.test("ConfigFile.diff returns array with additions and removals", async () }, ]); }); + +Deno.test("ConfigFile.diff reports inculde and exclude changes when one of the entries changed", async () => { + const config = await configFile.read( + fromFileUrl(new URL(import.meta.resolve("./config.json"))), + ); + assert(!!config); + + config.override({ include: ["foo", "bar"], exclude: ["fuzz", "bazz"] }); + + const changes = config.diff({ + include: ["fuzz", "bazz"], + exclude: ["foo", "bar"], + }); + assertEquals(changes, [ + { key: "exclude", addition: ["foo", "bar"], removal: ["fuzz", "bazz"] }, + { key: "include", removal: ["foo", "bar"], addition: ["fuzz", "bazz"] }, + ]); +}); \ No newline at end of file From eebcf1969622a5630ad0606b1f707d494a755317 Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Fri, 1 Dec 2023 13:27:37 +0100 Subject: [PATCH 8/9] fmt --- tests/config_file_test/config_file_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config_file_test/config_file_test.ts b/tests/config_file_test/config_file_test.ts index 3ab3473b..19551863 100644 --- a/tests/config_file_test/config_file_test.ts +++ b/tests/config_file_test/config_file_test.ts @@ -48,4 +48,4 @@ Deno.test("ConfigFile.diff reports inculde and exclude changes when one of the e { key: "exclude", addition: ["foo", "bar"], removal: ["fuzz", "bazz"] }, { key: "include", removal: ["foo", "bar"], addition: ["fuzz", "bazz"] }, ]); -}); \ No newline at end of file +}); From eb4065e7e07edfc6b81a8d398e8cfd2c582690cc Mon Sep 17 00:00:00 2001 From: Arnau Orriols Date: Fri, 1 Dec 2023 13:28:29 +0100 Subject: [PATCH 9/9] `Array.isArray` --- src/config_file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config_file.ts b/src/config_file.ts index 57e98253..4bdc07cc 100644 --- a/src/config_file.ts +++ b/src/config_file.ts @@ -90,7 +90,7 @@ class ConfigFile { // Iterate over the other args as they might include args not yet persisted in the config file for (const [key, otherValue] of Object.entries(otherConfigOutput)) { const thisValue = thisConfigOutput[key]; - if (otherValue instanceof Array && thisValue instanceof Array) { + if (Array.isArray(otherValue) && Array.isArray(thisValue)) { if ( thisValue.length !== otherValue.length || !thisValue.every((x, i) => otherValue[i] === x)