Defaulting to createEmpty if block map is empty in createWithContent (Fixes issue #2226)

[davoscript]

Summary

When passing createWithContent a state parameter created using convertFromHTML from an empty string and createFromBlockArray, as shown in the example:

const sampleMarkup =
  '<b>Bold text</b>, <i>Italic text</i><br/ ><br />' +
  '<a href="http://www.facebook.com">Example link</a><br /><br/ >' +
  '<img src="image.png" height="112" width="200" />';

const blocksFromHTML = convertFromHTML(sampleMarkup);
const state = ContentState.createFromBlockArray(
  blocksFromHTML.contentBlocks,
  blocksFromHTML.entityMap,
);

this.state = {
  editorState: EditorState.createWithContent(
    state,
    decorator,
  ),
};

The following error is thrown:

TypeError: Cannot read property 'getKey' of undefined

  84 | EditorState.createWithContent = function createWithContent(contentState, decorator) {
> 85 |   var firstKey = contentState.getBlockMap().first().getKey();
     | ^  86 |   return EditorState.create({
  87 |     currentContent: contentState,
  88 |     undoStack: Stack(),

The previous error is generated because createWithContent asumes that contentState.getBlockMap().first() will return an element, which is wrong for scenarios where the block map is empty. As a consecuence the getKey(); function in contentState.getBlockMap().first().getKey(); is executed on undefined, thus throwing the error.

The previous scenario is very common if you are planning to use the editor to allow a user to write HTML content which could also be blank, especially as a default value.

Solution

In order to make the less amount of modifications to the code, I've added a validation that will run createEmpty in case the block map is empty.

Test Plan

Open /draft-js/examples/draft-0-10-0/convertFromHTML/convert.html and change line #61 from:

const blocksFromHTML = convertFromHTML(sampleMarkup);

to:

const blocksFromHTML = convertFromHTML('');

No error will be shown.

[mrkev]

Sweet, thanks for the fix and the through explanation @davoscript! I'll try to merge it in the coming days.

[facebook-github-bot]

@mrkev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[davoscript]

That's great @mrkev, I see the Linter testing warning here, is there anything I can do to fix it? I'm happy to review it, but I can't see the details.

[mrkev]

Ah yeah @davoscript, we expect all if's to have curly braces (if (...) return ... vs if (...) { return ... }). Any chance you could fix this real quick?

[mrkev]

We should sync the lint rules on the ESLint config in this repo with the internal one to avoid these issues to be honest. For future reference we need to add:

Lint code: ESLINT
Lint name: curly

[davoscript]

Thanks a lot for your feedback @mrkev, I've just pushed the commit and see all tests passed this time ;)

[facebook-github-bot]

@mrkev has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

@mrkev merged this pull request in c42662e.