Making DraftBlockType less restrictive #1480
Conversation
- Making DraftBlockType less restrictive and allowing to distinguish between internal types and custom user types
- While landing "" I had to change the type of ContentBlock key from DraftBlockType to string to avoid internal conflicts, but due to the new less strict typing we can revert it back
mitermayer
force-pushed
the
issue-1453/fixing-too-strict-typing-for-block-types
branch
from
728e0ac
to
be51e0a
Compare
| @@ -26,7 +26,7 @@ const {List, Map, OrderedSet, Record, Repeat} = Immutable; | |||
| const EMPTY_SET = OrderedSet(); | |||
|
|
|||
| type ContentBlockConfig = { | |||
| key?: string, | |||
| key?: DraftBlockType, | |||
There was a problem hiding this comment.
Just adding back, had removed this temporarily while shipping the other PR internally, but now since DraftBlockType is more permissive we wont run in the same problem again
There was a problem hiding this comment.
My mistake in here, this should have been added to type not key but fixing it on a follow up PR #1485
There was a problem hiding this comment.
however we also want to retain a way to identify our core draft blocks.
I agree that we want to keep this strict in places where we don't expect custom user blocks. The only place inside Draft like that is the DefaultDraftBlockMap which uses DraftBlockMap type.
What do you think of creating a more specific type for our DefaultDraftBlockMap that only allows our core block types, and allowing the user-defined custom block map to use the looser DraftBlockMap type that includes CustomBlockType?
You had brought up the question of why this hasn't caused Flow errors internally, and I still wonder about that too. Can we look into that a bit before doing a release that includes this?
Also great use of type aliasing! :)
block types inference - Making sure DefaultDraftBlockRenderMap still references the internal supported block types
|
lgtm - thanks! :) |
There was a problem hiding this comment.
@mitermayer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: DraftBlockType needs to be less strict in order to allow user defined block types however we also want to retain a way to identify our core draft blocks. fixes: facebookarchive/draft-js#1453 Closes facebookarchive/draft-js#1480 Differential Revision: D6215628 fbshipit-source-id: e4647bf2bafe9adb7788740ec64c1445e307e632
Summary: DraftBlockType needs to be less strict in order to allow user defined block types however we also want to retain a way to identify our core draft blocks. fixes: facebookarchive/draft-js#1453 Closes facebookarchive/draft-js#1480 Differential Revision: D6215628 fbshipit-source-id: e4647bf2bafe9adb7788740ec64c1445e307e632
Summary: DraftBlockType needs to be less strict in order to allow user defined block types however we also want to retain a way to identify our core draft blocks. fixes: facebookarchive/draft-js#1453 Closes facebookarchive/draft-js#1480 Differential Revision: D6215628 fbshipit-source-id: e4647bf2bafe9adb7788740ec64c1445e307e632
DraftBlockType needs to be less strict in order to allow user defined block types however we also want to retain a way to identify our core draft blocks.
fixes: #1453