Feature request: visit() is not useable with modify()

[mcclure]

node-jsonc-parser presents several convenient ways of interacting: The scanner, the visit interface, or traversing a parse tree. Unfortunately, only the parse tree interface is compatible with modify/Edit/applyEdits. This means there are essentially three interfaces, but two are read-only.

Context

I have a lot of JSON files, from which I want to remove all instances of a particular deprecated property. I intended to use node-jsonc-parser to write a script to find those instances and delete them. At first I thought the Visitor interface would offer an incredibly simple way to do this; I could write a single JSONVisitor:

let edits:Edit[] = []
visit(jsonString, {
	onObjectProperty: function(property: string, offset: number, length: number, startLine: number, startCharacter: number) {
		if (property == "cursedPropertyName") {
			// Do something here to add "remove this property" to edits?
		}
	}
})
jsonString = applyEdits(jsonString, edits)

The realization I quickly hit was there is no way to create the Edit to move the property. modify() requires a JSONPath, obtaining a JSONPath (eg, findNodeAtLocation) requires providing a node or a root node (which means running the parse interface). I could create an Edit object manually with the offset and length of the property, but this might mean creating a noncompliant JSON document (eg, if I removed a property but not the preceding comma).

Expected behavior

There should be some way to use the convenient visitor-style interface with modify()/applyEdits(). Two ways I can think of to do this would be

1. Add a Node.visit() interface. The reason I would prefer to use the visitor interface rather than parseTree is parseTree required me to write code to recursively traverse the tree of node children, a somewhat complicated construction for a simple find/replace script. However, jsonc-parser could just as easily provide a visitor interface to node trees, calling a visitor function and passing in the appropriate node for each node in the DOM tree, eg calling onObjectProperty for each NodeType="property" node.

2. Add some variant of modify() that works with the offset/length arguments, but still knows how to produce edits that transform from a compliant JSON document to a compliant JSON document, eg, it also removes incidental material like whitespace and commas as appropriate. This option might be harder due to ambiguity about what to remove.

Another thing that would help would be simply being clearer in the documentation about what is supported. Writing my script once to use visitor and then starting over with parsetree was a bit frustrating, but if I had known to start with parsetree, I could have skipped the steps of trying to make it work with scanner and visitor and that would not have been frustrating. The documentation could make this clearer by, in the "Why" section, between the third and fourth bullet points, adding the non-bulleted text "Using parseTree enables the following extra functionality:".

[mcclure]

Actually, wait, could I have got this with getLocation()?

[Marcono1234]

The current API also appears to be rather inefficient for what you want to do. It appears for every removal you want to make, you would have to call modify() followed by applyEdits(). It does not appear to be possible to accumulate all edits from multiple modify() calls and then only call applyEdits() once because each edit will affect the location of subsequent edits.

Edit: Removed irrelevant part about JSON DOM processing from my comment.

[mcclure]

@Marcono1234 , I do not think that is accurate, the doc for applyEdits says:

"All offsets refer to the original state of the document. No two edits must change or remove the same range of text in the original document."

The first part seems to suggest to me a group of applyEdits may be applied atomically. The second sentence would only be necessary to support "apply many edits atomically". In my testing, I may submit multiple edits to a single applyEdits() and it applies them all in the right place.

[Marcono1234]

It seems to depend on which edits you are making. Here are two types which will produce malformed JSON:

• Removing subsequent array elements / object properties.
  This will cause both elements / properties to remove the same ,, which in the end results in one character too much being removed:

  const json = '{"a":1,"b":2}'
  let edits = jsoncParser.modify(json, ["a"], undefined, {})
  edits = edits.concat(jsoncParser.modify(json, ["b"], undefined, {}))
  
  // Prints only `{`; the closing `}` is missing
  console.log(jsoncParser.applyEdits(json, edits))

• Removing array elements / object properties in reverse order:

  const json = '{"a":1,"b":2,"c":3}'
  let edits = jsoncParser.modify(json, ["c"], undefined, {})
  edits = edits.concat(jsoncParser.modify(json, ["a"], undefined, {}))
  
  // Prints `{"b":2,"c":3`
  console.log(jsoncParser.applyEdits(json, edits))

There might be more cases.

[mcclure]

@Marcono1234: Interesting. But, because those examples seem completely valid per the node-jsonc-parser documentation, I believe you have found bugs and you should file those in their own issues.

[aeschli]

Yes, we don't have the functionality to compute the changes of multiple modifications at once.

[mcclure]

Can this be closed now that #62 is merged? I haven't looked into this close enough to work out whether 62 would have solved the problem I was having in August.

[Marcono1234]

Not completely sure if this is solved yet. Because this library does not support combining multiple modifications in a safe way set (as mentioned in #48 (comment)), you would probably still have to repeatedly use the visitor on the JSON document and then apply each modification, one by one.

You as reporter can probably judge best whether your use case is solved with the recent changed done by #62.
Unless @aeschli as maintainer wants to keep this issue open to keep tracking this feature request.