Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getNodePath/modify do not work for property nodes (capture too much content) #51

Open
mcclure opened this issue Aug 31, 2021 · 0 comments

Comments

@mcclure
Copy link

mcclure commented Aug 31, 2021

When run on a property node, getNodePath appears to instead returns a path to the enclosing dictionary of the property. This results in some serious problems:

  • findNodeAtLocation(tree, getNodePath(node)) is a different node from node.
  • Calling modify() on a property node removes additional content other than the property node targeted.

Repro steps

Check out the jsonc-parser-demo branch of this github repo.

Run these steps:

npm install && npm run build
node . -d transformBackground testcase.json > testcase-post.json

src/app.ts in this repro is a short script that processes a json file and removes all instances of the property whose key name is given as the -d argument, then prints the file out to STDOUT. To demonstrate the bug more clearly, it currently removes only the first instance before bailing out, and once it has identified the property to be removed it console.warns to STDERR (1) the node it intends to remove (2) the getNodePath of the target node and (3) the roundtrip result of findNodeAtLocation(tree, getNodePath(node)) on the target node. The important code in this example is the code immediately following the comment "PERFORM OPERATIONS HERE"; everything else only serves to load the file and walk the node tree.

My expected behavior running this test script on the included testcase JSON is that on STDERR the "DELETING" and "BUT REALLY DELETING" nodes will be the same (ie, findNodeAtLocation(tree, getNodePath(node)) should be a noop); and on STDOUT the single property "transformBackground" in the dictionary levels/0/screen will be removed.

My observed behavior is quite different:

  1. The path printed is [ 'levels', 0, 'screen' ]. In other words, as printed, it points to the enclosing dictionary, not the transformBackground property.
  2. The "DELETING" prints as expected the property I'm looking for, but the "BUT REALLY DELETING" is different and contains much more content— it does appear to encompass the entire dictionary stored under "screen".
  3. After applying the edits, jsondiff shows this change for the output file: [{"path": "/levels/0", "value": {"color": [0.3, 0.37, 1], "png": "level/test/trigger/trigger.png"}, "op": "replace"}]. In other words, it shows the "screen" property and its value were deleted from /levels/0.

Analysis

I am able to get the "expected behavior" I desire by, instead of passing at.node to getNodePath in the "PERFORM OPERATIONS HERE" section, passing in either of its children. If I do this I get the path [ 'levels', 0, 'screen', 'transformBackground' ] as expected and this is indeed the content that gets removed. However this feels wierd, and additionally it contradicts the documentation. The comment/doc for modify says: " * @param path The path of the value to change. The path represents either to the document root, a property or an array item." This implies modify should accept property nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant