feat: auto convert non-string content value to JSON string under text mode

[cloydlau]

Hello,

I received these issues cloydlau/json-editor-vue#11 & cloydlau/json-editor-vue#16 recently. Which all caused by using JSON value under text mode.

I'm wondering would it be a little bit weird if a JSON editor don't take JSON value and throw an Error?

My understanding is mode means writing mode and might have little to do with value type.

So I did a little change using the new parser option to stringify non-string content value under text mode rather than throwing an error. (lint & test passing)

It can be quite common to use JSON value under text mode. In which case users will have to stringify the content value themselves and therefore the content value will be stringified anyway.

[josdejong]

Thanks Cloyd for providing this PR and looking into this.

I'm not entirely sure if this is the right solution. Right now, you can provide two different content types: either { text: string } or { json: JSONValue }. It looks like you want to change this such that text can be JSONValue too? I think instead of that you can just use { json: JSONValue }?

[josdejong]

(I suppose we can improve the error message to explain about { json: JSONValue })

[cloydlau]

Oh, I just tried using content.json in text mode and it worked! I think I got confused by the document saying content.text is used when in text mode, I never thought JSONValue can be used in text mode. I need to change my code. I should choose to use content.text or content.json based on the type of value user provides.

[josdejong]

Yes indeed. The reason that there are two different content types for this is that if you receive a string, you cannot know for sure if it holds a stringified JSON document, or is a parsed JSON document holding just a string. The latter is a bit theoretical but I think it is good to have this explicitly modeled.

I'll make the error message more informative and see if I can improve the docs. Do you have a suggestion on how to change the text "used when in text mode" to make it less confusing in this regard? Maybe just leave it out?

[cloydlau]

I'm not sure about the 'hold' word you mentioned, do you mean we can't tell whether a string is a normal string like 'aaa' or a JSON string like '{"a":1}'? Maybe we can do:

if (typeof content === 'string') {
  if (content[0] === '{' && content.at(-1) === '}') {
    const jsonContent = parseAndRepairOrUndefined(content)
    if (jsonContent === undefined) {
      // content is a normal string
    } else {
      // content is a JSON string
      // we can treat it as JSON
    }
  } else {
    // content is a normal string
  }
} else {
  // content is a JSON
}

I believe a normal string starts with '{' and ends with '}' is very rare, so the performance should be ok.

[josdejong]

Yes indeed, it is an edge case, but a string by itself is also valid JSON. Trying to automatically detect whether a string was intended to be stringified JSON, or parsed JSON holding a string gives tricky edge cases. It will work in 95% of the cases I guess, but I prefer to explicitly model this to avoid any problems.

It is similar to say the HTML fetch() API, which has two explicit .text() and .json() methods, and not a magic method that will automatically determine whether or not to parse the contents as JSON based on certain heuristics (and I'm glad with that myself, that way I can count on it, I know for sure what the output is).

Modeling the contents as string | JSONValue instead of { text: string } | { json: JSONValue } is problematic: we cannot safely distinguish between the two since JSONValue itself can also be a string. It is not possible to statically determine whether the contents is either a stringified JSON document or a parsed JSON document. I think that is a bad design.

Thoughts about your proposal:

• A stingified JSON document can look like '{}', but also '[]' or can have whitespaces like ' {} '. That would be solvable in your example code of course.
• If the input is a string containing '[1,2,3', will you assume it is a parsed JSON document containing a string, or is it a stringified JSON document that needs to be repaired because the end bracket ] is missing? An automatic conversion step would silently interpret it as a parsed JSON document holding a string since the document doesn't end with ].
• When the input is a string containing 'true', will you assume it is a parsed JSON document containing a string, or is it a stringified JSON document holding the value true (which is valid JSON)? And the same with a numeric value like '123', is that supposed to be a numeric value or a string holding a numeric value?

It is tempting to hide this important difference (the contents being either stringified or parsed JSON) from the user, but I think the user needs to be aware of this. Other parts of the application will have assumptions about that too. If this difference is hidden from the user, it can be a tricky pitfall forgetting to parse/stringify in certain places, or accidentally stringifying JSON that is already stringified. What do you think?

[cloydlau]

Thanks for the detailed reply. I found this in the document: 'In case of tree mode, json is used. In case of text mode, text is used', It seems content[key] is indeed directly associated with modes in the first place. As you've said previously the correct reason is input should be explicit. I agree. So the difference of two should be like:

• content.text only accept strings, and will do JSON.parse
• content.json accept any value

If so, do we eventually need to distinguish them, why don't we unify them and let users to do the JSON.parse work if needed.
Will that be more explicit and simple?

I tried this example: content.text: '[1,2,3', now it will be automatically repaired and treated as array, what if it's mean to be a normal string?

[josdejong]

Thanks, good feedback.

I found this in the document: 'In case of tree mode, json is used. In case of text mode, text is used'

I see the documentation is confusion :) Let me try to improve the section in the docs that you're referring to:

content: Content Pass the JSON contents to be rendered in the JSONEditor. Content is an object containing a property json (a parsed JSON document) or text (a stringified JSON document). Only one of the two properties must be defined. You can pass both content types to the editor independent of what mode it has. When making a change in tree mode, the updated content will be { json }. When making a change in text mode, the updated content will be of type { text }. Please be aware that text can contain invalid JSON: whilst typing, a JSON document will be temporarily invalid (like when typing a new string), until the user is finished.

Is that a bit understandable? 😅 (if not please help out)

I tried this example: content.text: '[1,2,3', now it will be automatically repaired and treated as array, what if it's mean to be a normal string?

That is correct, it is repaired and parsed since context.text always contains a stringified JSON document, so the editor knows it should parse the contents. If you want to have these contents as a string, you can do that in two ways:
content.text: '"[1,2,3"' or content.json: '[1,2,3'. Does that make sense?

[josdejong]

If so, do we eventually need to distinguish them, why don't we unify them and let users to do the JSON.parse work if needed.
Will that be more explicit and simple?

Yes, so we could only allow content.text you mean and simplify the API in that case? That would be much simpler indeed. It is an option, however, it would be a problem for the performance: parsing and stringifying is slow (especially compared to not having to do such an action at all). The editor tries to minimize the need for it and "lazily" chooses the format that requires least amount of work. In the editor tree mode uses a parsed document internally, and text mode uses text internally. Assume you're working in a 200 MB document in tree mode. Adding a new item in an Array in memory can be done in milliseconds, but stringifying the full document takes 1 or 2 seconds or so. It would make the editor unusable for large documents when it has to stringify the document after every change. For small documents it would be fine though.

[josdejong]

I've improved the docs via f7f3837, feedback welcome!

[cloydlau]

I mean only content.json. Both content.json and content.text accept string, the only difference on string is there will be an extra JSON.parse work for the latter and users are not aware of this, right? They most likely use content.text simply because whose initial value is a normal string or under a text mode (after you updated the document they maybe won't). So why not leave the JSON.parse work to users if they want it explicitly.

[josdejong]

So you mean like only allow passing a parsed JSON document (content.json) and not allow passing a stringified JSON document (content.text)?

That would make things nice and simple indeed, and this can work for the tree mode (which internally uses a parsed JSON document). However, when using text mode, the content cannot always be represented as a parsed JSON document: whilst typing the document is (temporarily) invalid JSON, and also, any indentation and formatting is not part of a parsed JSON document. The state of the text mode needs to be text containing a stringified JSON document.

I'm not sure if we're yet at the same page. Does that make sense to you why we need both content.json and content.text in various circumstances, and why I would like to keep those two clearly separated?

[cloydlau]

😊 I understand content.text is indispensable in the code interior. The unification of content.json and content.text is only for exposed API which won't break any internal logic. What do you think?

[josdejong]

😊 I understand content.text is indispensable in the code interior.

👍

The unification of content.json and content.text is only for exposed API which won't break any internal logic. What do you think?

I tried to argue that also for the user of the library it is important to know whether dealing with a stringified or parsed document (instead of having a mix: sometimes being parsed and sometimes stringified). I myself would not unify the two content types. But if you think this is not a relevant thing for the users of json-editor-vue, you can indeed expose a simpler, unified API. That is up to you.

[cloydlau]

OK we are clear about this. Here's what I considered about json-editor-vue:

• Users precisely get what they passed, no JSON.parse or they can do that themselves if they want it explicitly.
• The uncertainty of sometimes being parsed and sometimes stringified comes from diverseness of mode. Users should eliminate that uncertainty by specifying a single mode.

[josdejong]

From your conclusion I see that I did a bad job with my latest updates in the documentation. I see you took the relation between mode and content type literally, where as I had only intended that explanation to illustrate why there are two different content types in the first place. My bad, I should have added words like "mostly" there.

It is not always the case that text mode equals { text } content, and tree mode equals { json } content. For example:

• When { json } contents is opened in tree mode, and the end user switches to text mode but does not make a change, the content still is { json }
• In tree mode, when the user clears the document (select all, delete), the content will be { text: '' }
• In tree mode with empty content, when the user pasts invalid JSON, the contents will be { text }

Here a next iteration of the docs, I moved most of the explanation to onChange:

onChange(...)
[...]
The returned content is sometimes of type { json }, and sometimes of type { text }. Which of the two is returned depends on the mode of the editor, the change that is applied, and the state of the document (valid, invalid, empty). Please be aware that { text } can contain invalid JSON: whilst typing in text mode, a JSON document will be temporarily invalid, like when the user is typing a new string.
[...]

[cloydlau]

When { json } contents is opened in tree mode, and the end user switches to text mode but does not make a change, the content still is { json }

But onChange is triggered by content changing, not mode switching. User still gets a certain return when he specify a certain mode (Except the edge cases you pointed out, that's my oversight👍).
Never mind, I just believe a 'mixture' of content.text and content.json wouldn't bring in uncertainty.
I respect svelte-jsoneditor for keeping those two separated.

[josdejong]

Ok clear.

@bestkolobok you may find it interesting too to read up on this topic. Not sure, but looking at vue3-jsoneditor it seems like the user too gets a mix of sometimes stringified and sometimes parsed JSON in the same variable. I think it's important to at least be aware of that.

[bestkolobok]

Hi @josdejong!

Thanks for the detailed explanation of the text and json content in this thread.
First, the vue3-jsoneditor was giving separate props for text and json content, which follows the logic you described. But at some stage I decided to make the component easier to use by making a common entry point for both text and json content (while leaving separate props for text and json content for backward compatibility with older versions). At the same time, I did not think about the possible confusion for the user that this would lead to. Also, currently the vue3-jsoneditor incorrectly determines the content type based on the data type: if the content typeof "string", then it is always passed as text, although, as you noted, this is not the case.

So I understand that the concept of binding the content of the view-editor needs some rethinking. At a minimum, I need to:

• Return to the documentation a description of individual props for json and text.
• Resolve the issue of user definability of the content type when using common binding and the description about it in the documentation.

Now, after a busy working day, there are still no constructive thoughts. I will try to make the appropriate changes within the next two days and let you know about it.

Thanks again for clarifying these important details.

[cloydlau]

I should clarify too that the unification of content.text and content.json is only a default behavior of json-editor-vue.
(I've made a description about it in the document)
User can absolutely be in accordance with svelte-jsoneditor:

<JsonEditorVue
  :content="content" :onChange="updatedContent => {
    content = updatedContent
  }"
/>

User can basically do whatever they can in svelte-jsoneditor in json-editor-vue.

[bestkolobok]

I made the following changes to vue3-jsoneditor today:

• Again allowed the use of separate v-model for text and json formats (v-model:json and v-model:text), and updated the documentation.
• Defined the data type for the base v-model depending on the mode tree or text
• Added an additional warning to the documentation about possible data type changes depending on the mode when using the base v-model

Despite the presence of two separate v-model for each type of data, I still decided to leave the option to use the base v-model, since such an implementation can be useful for many users.
Regarding the definition of the type of input data depending on the mode, I think that this is the most clear, since the type of output data in the svelte-jsoneditor in most cases also depends on the mode (except, as I understand, in one case when we delete in the editor all content). Although, of course, I may be somewhat wrong in my reasoning, you can correct me.

[josdejong]

@cloydlau that sounds good, I wasn't aware of :content="content", that sounds perfect: then the user can simply choose between the "simple" model or the "full" model depending on his/her needs 👍

@bestkolobok thanks for your updates, take it easy :). Your brief summary is correct:

the type of output data in the svelte-jsoneditor in most cases also depends on the mode (except, as I understand, in one case when we delete in the editor all content)