Fix bad destructuring when content block key has a -

[jankdc]

Summary

If a content block was created with a key that contains the -, it would fail to decode the decoratorKey and the leafKey because the current implementation relies on - to be a parsing delimiter. Here's an example:

class SomeComponent extends React.Component {
  constructor(props) {
    super(props);

    const block = new ContentBlock({
      type: 'paragraph',
      text: 'some-text',
      key: 'some-key'
    });

    const contentState = ContentBlock.createFromBlockArray([block]);

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

    this.onChange = editorState => this.setState({editorState});
  }

  render() {
    return (
      <Editor
        editorState={this.state.editorState}
        onChange={this.onChange}
      />
    );
  }
}

When you select anywhere on the editor, you'd get this error:

    TypeError: Cannot read property 'getIn' of undefined

      45 |   const focusBlockKey = focusPath.blockKey;
      46 |   const focusLeaf = editorState
    > 47 |     .getBlockTree(focusBlockKey)
         |                              ^
      48 |     .getIn([focusPath.decoratorKey, 'leaves', focusPath.leafKey]);
      49 |
      50 |   const anchorLeafStart: number = anchorLeaf.get('start');

      at getUpdatedSelectionState (src/component/selection/getUpdatedSelectionState.js:47:30)
      at getDraftEditorSelectionWithNodes (src/component/selection/getDraftEditorSelectionWithNodes.js:48:58)
      at getDraftEditorSelection (src/component/selection/getDraftEditorSelection.js:37:53)
      at assertGetDraftEditorSelection (src/component/selection/__tests__/getDraftEditorSelection-test.js:55:53)
      at Object.<anonymous> (src/component/selection/__tests__/getDraftEditorSelection-test.js:211:3)

Test Plan

Added a unit test for DraftOffsetKey.js to check for all delimiter cases.

[claudiopro]

Good catch @jankdc, thanks for contributing this fix to Draft.js! I can't find an issue associated with this PR, I'm curious how did you encounter this problem?

Please take a look at my comments inline, I think we can make the fix slightly easier to reason about.

[claudiopro]

Changes look good, thanks for adding a test case to show the problem and its fix.

Please take a look at my suggested changes to make the code easier to reason about.

[jankdc]

Oh sorry, I didn't really create an issue for it. We encountered this problem because we were manually mapping our tree data structure to a list of ContentBlocks , where each branch or leaf, had a GUID id, like so:

const data = {
  id: "0dca5e9d-2ebd-4574-921e-2b68793c9e8c",
  sections: [
    {
      id: "4b35e274-81e6-4eb5-8964-ef566517051d",
      text: "Some-text",
      meta: { ... },
      sections: [ ... ]
    }
  ]
}; 

function reduceTree(data): ContentBlock[] { ... }

The reason we're going this route is because we want to keep some metadata in the blocks instead of using convertFromHTML which strips it all away. Anyway, that's where we came across it.

[claudiopro]

Ah, that makes sense. Maybe we should make the separator token configurable. Or even better, allow arbitrary payload to be associated with blocks e.g. via data- attributes. Thanks again for contributing this fix!

[facebook-github-bot]

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

[jankdc]

Or even better, allow arbitrary payload to be associated with blocks e.g. via data- attributes.

Yeah, this would be a really handy feature.

[facebook-github-bot]

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

[SeanSilke]

Hello when this fix will be released? Do you Have "release circle", there is near three months past from last release.