feat: Support jsonc config files #206
Conversation
|
@magurotuna I've addressed your comments, PTAL. |
src/config_file.ts
Outdated
| 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 | ||
| const thisValue = (this.args() as any)[key]; | ||
| if (otherValue instanceof Array) { | ||
| const thisArrayValue = thisValue as typeof otherValue; | ||
| if (thisArrayValue.length !== otherValue.length) { | ||
| return false; | ||
| } else if (!thisArrayValue.every((x, i) => otherValue[i] === x)) { | ||
| return false; | ||
| for (const [key, otherValue] of Object.entries(otherConfigOutput)) { | ||
| const thisValue = thisConfigOutput[key]; | ||
| if (otherValue instanceof Array && thisValue instanceof Array) { | ||
| if ( | ||
| thisValue.length !== otherValue.length || | ||
| !thisValue.every((x, i) => otherValue[i] === x) | ||
| ) { | ||
| changes.push({ key, removal: thisValue, addition: otherValue }); | ||
| } | ||
| } else if (thisValue !== otherValue) { | ||
| return false; | ||
| changes.push({ key, removal: thisValue, addition: otherValue }); | ||
| } | ||
| } | ||
| return true; | ||
| return changes; | ||
| } |
There was a problem hiding this comment.
The logic of diff function now supports array and primitive types, but iterating over Object.entries might be conveying that this logic is generic enough to cover all the types (including objects), which makes me a little bit confused.
As far as I understand there is no case in reality where otherConfigOutput has unknown keys other than ones defined in ConfigArgs because of what normalize method does, but I'm just wondering if there's a good way to avoid confusion.
(I'll approve it as-is anyway :)
There was a problem hiding this comment.
I like using Object.entries as I prefer the possible false-negatives resulting from it over the false-positives that we would risk if we explicitly checked each property. The reason being that false-negatives are more visible hence easier to detect.
Resolves #198