Assume block depth of 0 if not specified

[tpict]

I'm having an issue where DraftJS will generate blocks with no depth key at times, for example

{
  "blocks": [{
    "key": "fvkqt",
    "text": "hello world",
    "type": "unstyled",
    "inlineStyleRanges": [],
    "entityRanges": [],
    "data": {}
  }],
  "entityMap": {}
}

and parts of this library that assume a depth key to be present choke.

Perhaps this is a bug in the client library (I'm on 0.10.5), but I thought I'd open a PR anyway as I'd really like to resolve the issue quickly :sweat_smile:

Thanks for putting together this library, it could really accelerate development of some features we're working on :slightly_smiling_face:

[thibaudcolas]

Hey @tpict, glad to know the library is useful to you!

This change looks good to me at a high level, I’d just want to check whether there might be other places where depth is used, or where we might be relying on other attributes that might go missing.

I haven’t come across cases where depth would not be set though, do you have further details on how that happens?

[tpict]

hi @thibaudcolas, sorry for the false alarm but I think I've tracked this down to a bug in https://github.com/jpuri/html-to-draftjs (I thought my code was bypassing it but I'd messed up a conditional somewhere). Draft.js's convertToRaw seems to ignore any missing properties in the input ContentState. Maybe there's a case then for validating the JSON shape in this library?

[thibaudcolas]

👌I see. I think it would be appropriate for the exporter to handle content like convertFromRaw does – so in this case, treating a block without depth as if it was depth: 0. If that sounds good to you I'll proceed with reviewing, merging, & releasing this as a patch release.

I like the idea of having a way to validate the shape of the JSON, but I wouldn't want this to be done in the exporter.render API because of the performance impact. If you feel like it would be valuable to you, we could add this as a separate API of the exporter (e.g. exporter.validate(content)?) that could be used separately from exporter.render, in places where performance isn't a concern. It feels like this could almost be its own library, but I can see why it would be valuable for this to share a similar configuration to the exporter’s, and the audience would most likely be the same.

If that sounds like something you'd like to see happen, the first step would be to open an issue for it as a feature request, describing some of the use cases, and potential solutions, then make a PR with the implementation as a new API that wouldn't cause performance of backwards compatibility concerns for exporter.render().

[tpict]

Thanks @thibaudcolas – sorry for not responding earlier, I've moved on from the job in which I was using Draft.js! Still, I'll think about ContentState validation, I think that would be a really useful feature for client serializers etc.