Slate: nodeProperties function returns pseudo-symbol type identifier

Created on 24 Aug 2017  路  5Comments  路  Source: ianstormtaylor/slate

Below I have this situation, two Paragraph blocks, one containing an inline void Image, and another containing only text. Here is what happens when I attempt to drag and drop it.

screen shot 2017-08-24 at 11 01 53 am

Dragging it causes the onDrop to fire, which will have a data.type of Node, and contain the inline void image node in data.node. This will cause onDropNode to fire which will then go on to insertBlock

screen shot 2017-08-24 at 11 02 18 am

The insertBlock transform will then call normalize on the node being passed in.

screen shot 2017-08-24 at 11 02 49 am

In the normalize, this node will fail the Block check, because it's an Inline. Falling through, we pass it through nodeProperties, and then shove it into Block.create

screen shot 2017-08-24 at 11 03 03 am

It seems as though nodeProperties, creates an object that has the pseudo-symbols from not just us, but also immutableJS

screen shot 2017-08-24 at 11 10 45 am

This causes some crazy stuff to happen. Now we are at a point, where theBlock.create function is tricked into thinking what it has is an Inline node, however what it actually has is just some properties. It quietly returns the properties, and then all hell breaks loose

screen shot 2017-08-24 at 11 04 08 am

It manages to make it's way from insertBlock all the way to insertNode. When it gets there, it attempts to mapDescendants. Unfortunately, we aren't an Inline that has a nodes property.
So we just actually die here.

screen shot 2017-08-24 at 11 12 42 am

@ianstormtaylor I see a couple of spots where this should be fixed, but in my opinion the true source of pain is nodeProperties. Should we be white listing what properties we let through?

bug

All 5 comments

@YurkaninRyan great writeup, makes the problems super clear.

  • [x] On the #836 branch I've actually got all of the Model.create methods using white listing the properties that the models are aware of. I realize this doesn't help as soon as possible, but I'm hoping to merge this branch in soon and do a bump.

It sounds like we've also got at least:

  • [ ] The onDrop handler should be smart enough to use insertBlock or insertInline.

  • [ ] The Normalize.block/inline/text functions should throw an error if they are working with one of the other kinds of nodes, instead of doing crazy stuff.

Maybe some others that you're aware of too. Let me know! I'd love PR's that help to address any of these pieces. Thank you!

You got it, I'll tackle this one, and get those two other items in there!

@YurkaninRyan I just ran into something like this on another branch, except with weird zombie Mark objects instead of Inline objects after being passed through Normalize.mark. Do you know why those were being created? I'm about to dig in, but figured you may have already figured it out. Thanks!

@ianstormtaylor I do not :(, but i'll probably end up encountering this soon because I plan on spending the next few days hunting these inconsistencies down

@YurkaninRyan going to close this since the root issue was solved. If we notice that weird behavior happening again we can open a new issue to solve whatever it was.

Was this page helpful?
0 / 5 - 0 ratings