comments lost while merging

[wmertens]

We're using your project in Qwik to merge vscode settings, and in our unit tests the comments are gone.

This is the code: https://github.com/QwikDev/qwik/blob/54215ef5d026f52f4c3c257b80f8e21071749bae/packages/qwik/src/cli/add/update-files.ts#L117-L122

This is the unit test, notice the missing // Comment in the result: https://github.com/QwikDev/qwik/blob/2f31c3a2b88809680416dc61291ea01489d6ac20/packages/qwik/src/cli/add/update-files.unit.ts#L123

Are we doing something wrong or is this expected?

[marcospassos]

When you spread objects and convert them to JSON, you end up discarding all token information and trivia, including things like formatting and comments:

destPkgJson.update({ ...destPkgJson.toJSON(), ...srcPkgJson.toJSON() });

To preserve the original formatting and make minimal changes, you need to work directly with the CST.
Here's one way to do it:

const srcPkgJson = JsonParser.parse(srcContent, JsonObjectNode);
const destPkgJson = JsonParser.parse(
  await fs.promises.readFile(destPath, 'utf-8'),
  JsonObjectNode
);

for (const property of srcPkgJson.properties) {
  destPkgJson.set(property.key, property.value);
}

console.log(destPkgJson.toString());

If you'd rather discard the formatting from srcPkgJson so the algorithm can "learn" from destPkgJson and apply its style to the new nodes, you can reset it before merging:

const srcPkgJson = JsonParser.parse(srcContent, JsonObjectNode);
const destPkgJson = JsonParser.parse(
  await fs.promises.readFile(destPath, 'utf-8'),
  JsonObjectNode
);

srcPkgJson.reset();

for (const property of srcPkgJson.properties) {
  destPkgJson.set(property.key, property.value);
}

console.log(destPkgJson.toString());

Let me know if this fixes the issue for you

[wmertens]

Ok thank you! Maybe this should go into the readme?

[wmertens]

actually, this doesn't copy comments from the source, does it?

[gioboa]

Hi @marcospassos thanks for this lib.
I tried with for (const property of srcPkgJson.properties) { but with this we are stripping srcPkgJson comments as well.

[marcospassos]

This is a more advanced case, where you want to preserve comments from both documents.

Here's how I'd implement this:

import {
    JsonNode,
    JsonObjectNode,
    JsonParser,
    JsonPropertyNode,
    JsonTokenNode,
    JsonTokenType,
} from "@croct/json5-parser";

function isTrivia(node: JsonNode): boolean {
    if (!(node instanceof JsonTokenNode)) {
        return false;
    }

    return node.isType(JsonTokenType.LINE_COMMENT)
        || node.isType(JsonTokenType.BLOCK_COMMENT)
        || node.isType(JsonTokenType.NEWLINE)
        || node.isType(JsonTokenType.WHITESPACE);
}

function merge(source: JsonObjectNode, destination: JsonObjectNode): JsonObjectNode {
    // Remove any existing destination properties that clash with source,
    // along with their leading/trailing trivia, to avoid duplicate keys
    for (const property of source.properties) {
        const key = property.key.toJSON();
        const index = destination.children.findIndex(
            (node) => node instanceof JsonPropertyNode && node.key.toJSON() === key,
        );

        if (index >= 0) {
            let startIndex = index;
            let endIndex = index;

            // Find leading trivia
            while (startIndex > 0 && isTrivia(destination.children[startIndex - 1])) {
                startIndex--;
            }

            // Find trailing trivia
            while (endIndex + 1 < destination.children.length && isTrivia(destination.children[endIndex + 1])) {
                endIndex++;
            }

            // Trim tivia
            destination.children.splice(startIndex, endIndex - startIndex + 1);

            // Delete overriden keys
            destination.delete(key);
        }
    }

    // Add thew new properties
    for (const property of source.properties) {
        destination.set(property.key, property.value);
    }

    // Add all trivia nodes to preserve comments, newlines, spaces, etc (during reconciliation)
    destination.children.push(...source.children);

    return destination;
}

Usage:

const destinationCode = `{
    // Destination pre-foo comment
    "foo": "value",
    // Destination post-foo comment
    "baz": [1, 2, 3]
}
`;

const sourceCode = `{
    /* Source pre-bar comment */
    "bar": 123, /* Inline comment */
    /* Source post-bar comment */
    "baz": true /* Another inline comment */
}
`;

const source = JsonParser.parse(sourceCode, JsonObjectNode);
const destination = JsonParser.parse(destinationCode, JsonObjectNode);

console.log(merge(source, destination).toString());

Result:

{
    // Destination pre-foo comment
    "foo": "value",
    /* Source pre-bar comment */
    "bar": 123, /* Inline comment */
    /* Source post-bar comment */
    "baz": true /* Another inline comment */
}

This should cover your use case.

If you'd like, I'd be happy to accept a PR that adds first-class support for this via a JsonObjectNode.merge method.

[gioboa]

Thanks @marcospassos
I'll try this implementation soon.

[marcospassos]

Just released 0.2.0 with this feature. Let me know if it resolves the issue.