Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

'blockRenderMap' may cause unhandled exception when 'blockRenderFn' is provided #1211

Closed
southerncross opened this issue May 15, 2017 · 3 comments

Comments

@southerncross
Copy link
Contributor

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

I want to create a block type to center text, so I defined a new block type 'CENTER_BLOCK_TYPE' and a new blockRenderFn.

But I got an error: Uncaught TypeError: Cannot read property 'wrapper' of undefined.
(Draft 0.10.1)

image

The online example is: https://jsfiddle.net/m6z0xn4r/191/

After debugging for a while, I noticed that it was the blockRenderMap that caused the exception.

const configForType = blockRenderMap.get(blockType); // 'configForType' will be undefined
const wrapperTemplate = configForType.wrapper; // can not read wrapper of undefined

(https://github.com/facebook/draft-js/blob/master/src/component/contents/DraftEditorContents.react.js#L147)

Since I didn't provide customized blockRenderMap, so the exception occurred when Draft tried to find out the wrapper.

I think it is not a normal behavior, because according to Draft's doc:

Any block that is not recognized by the block redering mapping will be treated as unstyled

So I think the code should be changed as:

const configForType = blockRenderMap.get(blockType) || blockRenderMap.get('unstyled'); // add a default value
const wrapperTemplate = configForType.wrapper;
@hellendag
Copy link

That seems like a reasonable solution. Feel free to send a PR, it sounds like it'll be a simple one to merge.

@southerncross
Copy link
Contributor Author

No problem 😄

flarnie added a commit to southerncross/draft-js that referenced this issue Sep 20, 2017
**what is the change?:**
Adding a test for the solution to issue facebookarchive#1211

In general I prefer testing the public API, so we actually render an
example editor and trigger the situation which used to throw an error.

**why make this change?:**
We need better unit tests for Draft, and this will help stop the bug
from recurring if someone randomly removes that check in the future.

**test plan:**
`yarn run test src/component/contents/__tests__/DraftEditorContent.react-test.js`

We tried temporarily undoing the fix and running the test, and verified
that it failed.

**issue:**
facebookarchive#1211
facebook-github-bot pushed a commit that referenced this issue Sep 22, 2017
Summary:
**Summary**

PR for issue 1211
#1211

**Test Plan**

<s>no need</s>
**Edit by Flarnie:** added unit test
Closes #1302

Reviewed By: zpao

Differential Revision: D5881334

Pulled By: zpao

fbshipit-source-id: b8d57b23a1654fe9cdcf84b74b25617413ef6248
@mitermayer
Copy link
Contributor

This issue has been already fixed by 3da87b5

midas19910709 added a commit to midas19910709/draft-js that referenced this issue Mar 30, 2022
Summary:
**Summary**

PR for issue 1211
facebookarchive/draft-js#1211

**Test Plan**

<s>no need</s>
**Edit by Flarnie:** added unit test
Closes facebookarchive/draft-js#1302

Reviewed By: zpao

Differential Revision: D5881334

Pulled By: zpao

fbshipit-source-id: b8d57b23a1654fe9cdcf84b74b25617413ef6248
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants