Changing customStyleMap doesn’t trigger re-render until contentState is changed

[cbothner]

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

What is the current behavior? After customStyleMap is changed, nothing happens until contentState is changed.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem..

Check it out here: https://jsfiddle.net/m6z0xn4r/125/

What is the expected behavior?

52 suggests that this exact problem has been cleared up. Regression?? Or am I not understanding how to do the thing?

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js? Latest, macOS, seems like all browsers. Dunno about previous versions.

[cbothner]

I don't mean to be annoying, but this is blocking me at work. I have tried to find a workaround but have come up short. Any thoughts about alternative solutions would be appreciated, as would some indication of the timeline for this bugfix.

[fisx]

i am also struggling with this. thanks :-)

[cbothner]

I ended up just having to trick draftjs into thrashing the editor dom. I put this in my componentWillReceiveProps method and implemented _shouldJiggle to check the props that would change the style.

    if (this._shouldJiggle(nextProps)) {
      const contentState = editorState.getCurrentContent()
      const blockMap = contentState.getBlockMap()

      const indented = blockMap.map(blk =>
        blk.set('depth', blk.getDepth() + 1))
      const outdented = indented.map(blk =>
        blk.set('depth', blk.getDepth() - 1))
      const outdentedContentState = contentState.set('blockMap', outdented)
      editorState = EditorState.set(editorState, {
        currentContent: outdentedContentState,
      })
    }

[fisx]

wait, what? why does that help? shouldn't it hold that outdentedContentState === contentState?

anyway, glad to hear you've found something, i'll look into it. thanks!

[cbothner]

The two objects have the same contents after this transformation but because draft-js is built on immutable objects they are actually not ===. Therefore, I would be careful to allow this to fire only when needed since it makes a lot of copies and that's never good for performance.

[fisx]

oh i see, thanks again.

(i am touching draft through a haskell binding in which the comparison works on the values not pointers, or at least that's what i hope. javascript confuses me.. :-)

[fisx]

Ok, I've found a couple of different ways of making the EditorState in the component props different so that the component should re-render. And it does. But the changes to the style map that I pass do not have any effect. I can see it rerender and show the mutated text, but the styles do not change with the text.

@cbothner do you happen to have another jsfiddle that works as we want it to?

[fisx]

Fixed it! The branch is git+https://github.com/liqula/draft-js.git#999-fix-leaf-shouldupdate.

PR coming up soon.

[fisx]

I patched the branch a little more, and now it actually does fix the problem. The PR may take a few days, though, please up-vote this if you want to make me go faster!

[slavikhard]

+1

[fisx]

Ok, i was too optimistic up there, and then moved on to other business. Sorry!

@slavikhard would you be willing to make it a PR yourself? I forgot the context and what's missing. If you need me to get it done I'll see what I can do.

[tomaskikutis]

Using Draft 0.10.5 it won't trigger a re-render even if contentState is changed. It seems now a block needs to be changed in order for styleMap to take effect for that single block. https://jsfiddle.net/mrxL6kq3/21/

[fisx]

as i said, it's all paged out of my brain currently, but this sounds like draft-js is just broken for the sake of performance? i hope i'm wrong, though. facebook usually does a really good job with react stuff.

anybody from draft-js dev?

[flarnie]

In the earlier issue where this was mentioned, #52 Isaac had commented:

You can define a new customStyleMap to pass in as the prop, though that won't currently trigger a re-render of the entire editor if the ContentState hasn't changed along with it.

What we could do is make customStyleMap an immutable Map (which we probably should have done anyway), then allow a full re-render if it changes so that the newly defined styles can be picked up. That way, when you want to load your fonts, once they are available you could create a new customStyleMap and pass it through.

It looks like the actual issue was about a different problem which was resolved, but nobody made the change to trigger a re-render on changes to the customStyleMap that I know of. So someone could still submit a PR for that, sounds reasonable to me.

Changing the customStyleMap is not a common use case for us at Facebook, so we have not run into this problem, hence the current behavior. Would be great to get it fixed though! :)

[fisx]

thanks @flarnie, this is very helpful! :-)

unfortunately, this issue is deep down my list at the moment.

@slavikhard as you seem to be interested in this, is there anything that's missing from https://github.com/facebook/draft-js/compare/master...liqula:999-fix-leaf-shouldupdate that you would need from me in order to be able to make a PR?

Here's the relevant commits. You probably want to drop 8c16719ca5a7e2559bad548eece2e1e00e7032f2 to make it easier to read / rebase.

git log -r 7b1752581876a5061f738bce29e87bede516c498..HEAD

I'll keep reading this, but I won't be able to do much more than comment. Good luck! (-: