Parsing JSON with duplicate keys in onChange throws "RangeError: Selection points outside of document"

[ZRui98]

Hi, I ran into this with svelte + svelte-jsoneditor v0.17.9.
When pasting in JSON with duplicate keys, this error from codemirror gets thrown: Uncaught (in promise) RangeError: Selection points outside of document. I noticed this started occurring on v0.17.9, but was not present on v0.17.8.
Minimal example to reproduce: https://www.sveltelab.dev/8u8l783v8eij2b1
To reproduce, paste

{"a":"a", "a":"b"}

into the editor. An error should be thrown in the console log.
Thanks.

[josdejong]

Thanks for reporting.

I can reproduce the same error with the following contents:

{"a":"a"                }

The issue occurs when you replace the JSON text contents with something that was shorter than the original text during in the onChange callback. In your case, you run JSON.parse followed by a JSON.stringify($json, null, 2). This loses the original white spacing and duplicates.

We should look into why this error is thrown, that should not happen.

At the same time: you should not run this JSON.parse and JSON.stringify loop in the first place. That makes it impossible to edit the contents in a "normal" way, since the editor is normalizing all white-spacing whilst you're typing. For example: initially, the editor in your example contains {}. When I put the cursor between the { and }, and press enter to start typing a new property on the second line, the contents is reset again to {}, making it literally impossible to insert something.

Instead, you can just keep the content in your state, and only as soon as you're going to save the document or so, run a JSON.parse on the contents.

[josdejong]

Fixed now in v0.18.2

[hybridwebdev]

This is still an issue in 0.23.8. Why close the ticket and claim it's fixed when the issue hasn't been fixed?

Issue remains exactly as OP explained.

Specifically in vanilla-jsoneditor.js you have code like this:

function checkSelection(selection, docLength) {
  for (let range2 of selection.ranges)
    if (range2.to > docLength)
      throw new RangeError("Selection points outside of document");
}

Why would you choose to throw an error. This causes the entire editor to crash and stop functioning. This is incredibly bad practice. The better solution would be to throw a warning and null the selection, which would fail gracefully instead of crashing the editor.

[josdejong]

@hybridwebdev you sound offensive to me and the author of the code that you mention (part of codemirror). Please adjust your tone, then we can look into this.

This error being thrown is totally fine in my opinion. We should not pass an invalid selection in the first place, so if that (still) happens, we simply need to address that, right?

[hybridwebdev]

@hybridwebdev you sound offensive to me and the author of the code that you mention (part of codemirror). Please adjust your tone, then we can look into this.

This error being thrown is totally fine in my opinion. We should not pass an invalid selection in the first place, so if that (still) happens, we simply need to address that, right?

No tone was intended. Huge fan of this repo. In fact I've contributed pull requests. Just pointing out the issue clearly still exists.

The cause of the issue is caused by a difference in the string value of the content. The editor constantly tries to "normalize" the string contained.

Consider this pseudo vue code:
Script

let myState = defineModel("modelValue", {
   foo: "hello"
})
let text = computed( () => JSON.stringify( myState, null, 4 ) )

let content = { text }

Pretend there's more logic here, but essentially just assuming the editors state is valid
JSON, therefore updating the reactive state with the current editor state.

let onChange = v => myState.value = JSON.parse( v.text )

Template
<vanilla-json-editor :content=content @onChange=onChange>

MyState is a reactive v-model. The passed in value is a standard JS object. When the editor changes, it fires the onChange event which the component uses to update the passed in state to reflect the changes in the editor.

However, you run into issues like OP had. If you enter a duplicate key name, when the normalization in the editor is run on the next tick, you end up with the key removed because JSON.parse removes duplicate keys. This creates a shorter string length, causing the editor to throw the error.

As I said, deliberately crashing the editor with that throw seems inappropriate. The better solution in that case would be just to null out the selection pointer if it's no longer in a valid range. This would cause it to fail gracefully instead of catastrophically. From a users perspective they'd just have to re-click the point they were at and continue editing.

[josdejong]

No tone was intended.

👍 ok thanks.

I'm trying to reproduce your issue. I took the original https://www.sveltelab.dev/8u8l783v8eij2b1 using v0.17.9, and upgraded this to use v0.23.8, see: https://www.sveltelab.dev/qyaa7n6nm52pmaw. When I paste {"a":"a", "a":"b"} as contents, I do not get any error, but the onChange event fires whilst that shouldn't be the case. Is that the issue you mean? . Because the onChange event triggers after pasting some content, the content is parsed and stringified again by the logic in the example (not in the editor), that makes that the contents is directly formatted and the duplicate removed.

How can I reproduce the issue you see?

[josdejong]

Any feedback @hybridwebdev ?

[hybridwebdev]

Sorry, kind of forgot about this due to work load.

Your example isn't really accurate to the issue, as your example is in svelte. As stated in my post, this issue pertains to Vue. Specifically Vue 3. Your example is somewhat accurate in the conditions that cause the issue in Vue. I believe something in Vue's reactivity system is causing a condition where the 2 "states" become out of sync, causing what's in the editor to shift against what's passed as an updated state through a reactive prop. I imagine this specifically happens in the microtask between the change and when nextTick is fired. This causes the condition where the pointer shifts outside of where it should be, due to the JSON.stringify tossing out duplicate keys.

Regardless of the cause or framework involved, the specific issue here is that the handling of this condition is being done poorly. As I stated, it would be better to null out the selection in the case of it being invalid, instead of catastrophically crashing the editor.

[josdejong]

Thanks for your explanation @hybridwebdev. I would love to figure out the underlying issue and see if we can get that fixed.

Do you have a (minimal) vue 3 example demonstrating the issue?

[hybridwebdev]

Thanks for your explanation @hybridwebdev. I would love to figure out the underlying issue and see if we can get that fixed.

Do you have a (minimal) vue 3 example demonstrating the issue?

My implementation is part of a very complex and private project. I'll see if I can put together a pen when I have a free moment.