Stuck in endless loop (crashes browser)

[roundrobin]

Hey Guys,

I'm using the convertToHTML method and I managed to get stuck into an endless loop with it, that crashes the browser. Just wanted you to be aware of it.

This is how I used it:

import{ convertToHTML } from "draft-convert";
const html = convertToHTML(this.state.editorState.getCurrentContent());

The only special thing I do before calling this method was to initialize Draft.js with a raw document ({blocks: [...], entityMap: {}}) that I had stored in my DB (contains bulletpoints). Like this:

var jsonFromDB = {blocks: [...], entityMap: {}};
var blocks = convertFromRaw(jsonFromDB);
var editorState = EditorState.createWithContent(
     blocks, decorator
);

this.setState({
    editorState: editorState,
});

[benbriggs]

Does this happen with very basic editor contents? What kinds of functionality are in your editor? Is there a specific operation it repeats? There are about 100 tests that run and pass before publishing a version, many of them end-to-end, so I'm going to need more information to narrow down what's going on exactly.

[roundrobin]

yeah sorry for not being super helpful yet. I'm going to take the time to reproduce this issue in a JSFiddle next week. My hunch is that the while-loop in convertToHTML is never exiting in this scenario.

[roundrobin]

@benbriggs I put together this gist that reproduces the issue. Just click the "click me" button, i'll cause the infinite loop. I confirmed this by adding some logs into "convertToHTML" method and saw it cause the logs fill up.

https://gist.github.com/roundrobin/a9ef94be7d1da316fe519b7127df992b

Lemme know if this is helpful to you.

[roundrobin]

Here are the versions of the packages I'm using:

{ 
"draft-convert": "^2.0.1",
"draft-js": "^0.10.4",
"react": "^15.6.2"
}

[benbriggs]

@roundrobin took a look and I think i've identified the issue - the serialization of the ContentState turns the depth value on each block into a string instead of keeping it as an integer. It seems Draft.convertFromRaw doesn't do any checks around this so it stays as a string when turned into an Immutable object. Since convertToHTML is being passed an invalid ContentState I don't think this is an issue with draft-convert - feel free to either open an issue with draft-js and/or in the meantime parse out the depth values to integers. Thank you for the detailed follow-up!

[roundrobin]

hm did you confirm this is the interface Draft.js expects? Could be they allow string and integer.

Otherwise I would suggest why not parsing the string into an integer and add a check to the while if the passed in depth is a proper int?

[roundrobin]

otherwise it seems like a very bad user experience of this library to let users run into a infinite loop and have their apps crash.

[roundrobin]

Relevant: https://github.com/facebook/draft-js/issues/1544

[benbriggs]

their flow typings for ContentBlock show that they expect depth to be a number. agreed that if this were a format by draft-js we should parse it, but it appears to be invalid.

[roundrobin]

sure I agree with you, but shouldn't you at least thrown an exception before letting the user run into an infinite loop?

[benbriggs]

i think it'd be odd to throw one for that instance without validating the entire structure (which i think is pretty far outside the charter of this project). additionally, i'd expect that these differences are going to cause issues not just with draft-convert but with core draft-js functionality, such as this operation. i think its fair for this project to make the same amount of assumptions as draft-js and have undefined behavior for invalid input.