Slate: Proposal for Solution to Force Rerender/Selective Render of Nodes

Created on 24 Nov 2017  Â·  11Comments  Â·  Source: ianstormtaylor/slate

Issue

One issue I find appearing in multiple use cases is how to have editor state outside the nodes force rerendering of nodes.

Here are two use cases that can't be handled elegantly right now:

  • Editor has an editing mode: For example, the editor has readOnly=false but that doesn't mean we are editing right now. When the user clicks in the editor, we turn on editing mode. When you are in editing mode, the image blocks, for example, might show a "delete" button. This "delete" button should not show when you aren't editing.

  • Highlight blocks: We want to highlight blocks depending on the block keys in an Array. highlightedBlocks=['abcde', 'fghij']. The list of highlighted blocks is an Array outside of the nodes. If the highlighted blocks change, the editor doesn't know and therefore doesn't change the rendering of the highlighted nodes.

One issue that was solved in another manner is the re-rendering problem when switching both from and to readOnly mode. My proposal will also discuss an alternate solution that can encompass readOnly mode as well.

https://github.com/ianstormtaylor/slate/pull/544

Proposal

I feel like a bad solution would be to continue to change shouldNodeComponentUpdate and keep passing through new values whenever new use cases appear. This is what was done to solve the readOnly problem.

My proposed solution is to pass in one new value through which is the value object itself. The value object has a data property that we can use to send additional values. I originally thought the solution might be to pass through props or something like that but the beauty of value is that it forces us to ensure that the solution we use can fit into value which is Immutable which means that Slate continues to stay functional as much as possible.

So in our case of editing mode, when somebody enables editing mode, we set value by doing something like:

const newValue = value.setIn(['data', 'isEditing'], true)

Now, because value would be passed around through the nodes, we can use the plugins shouldNodeComponentUpdate to force re-render;

function RerenderOnIsEditing() {
  return {
    shouldNodeComponentUpdate(props, nextProps) {
      if (props.value.isEditing !== nextProps.value.isEditing) return true
    }
  }
}

In order to get readOnly to work in the same manner, we can take one of two approaches:

  1. We could build a compatibility layer where if props.readOnly differed from value.data.readOnly then we immediately call the onChange with value.data.readOnly=true to fix that.

  2. This might be better for the future, is we can have the outer component do this where basically instead of passing in a props.readOnly we have them set value.data.readOnly. The reason why the latter might be preferable is because it gets users into the habit of storing all editor state in the value. This makes it obvious that when they want to add, for example, isEditing to the state, it should go in the same place that readOnly already exists. The discoverability would be better at the cost of a bit of ease of use.

idea

Most helpful comment

Hey @thesunny,

Any chance you can share how you did it?

In my renderNode function, I render some nodes depending on the previous and next block type. So when changing a node type, the previous and next one should be re rendered.

Despite the awesome documentation, I was not sure how renderNode was called.
If I am right, the nodes rendered are the node focused AND the node who lost the focus.
Maybe adding that info to the doc could be helpful.

All 11 comments

Just wanted to say, if you like this proposal, I can create a pull request.

Sorry to add to this so quickly but I also noted that if we pass the value through, we could technically remove many of the other props passed to rendering the node:

  • editor
  • isSelected REMOVABLE
  • node
  • parent REMOVABLE
  • readOnly REMOVABLE
  • value ADDED

Not saying we should remove them but might be something to consider. with value, the rest of the props can be calculated.

I think it may be worthwhile to still keep them around for performance/convenience; however, if it feels like we keep arbitrarily adding new props, maybe better to go cleaner and just stick with the ones we absolutely need.

Hey @thesunny thanks for writing all of this up!

I'm not sure about this. Removing some of the props like isSelected, parent, etc. are passed through to save performance since they are calculated as the tree is rendered.

But further, I'd like to keep value.data as a purely user-land space. And putting things like readOnly into them defeats that purpose, and starts to special case things.

To me it seems like something that should be handled using react-broadcast instead? Could you check that out and let me know if that would solve your issue? Or if not, how come?

Thank you!

Hi @ianstormtaylor,

Thanks for the response.

I found a way to do this purely in SlateJS using the plugin architecture. If I have time, I'll write up how I did it. For now, I'll close thie issue.

Hey @thesunny,

Any chance you can share how you did it?

In my renderNode function, I render some nodes depending on the previous and next block type. So when changing a node type, the previous and next one should be re rendered.

Despite the awesome documentation, I was not sure how renderNode was called.
If I am right, the nodes rendered are the node focused AND the node who lost the focus.
Maybe adding that info to the doc could be helpful.

I have a similar issue and still looking for a solution… I had hoped that changing data on the document might work but then ran into the above.

It would be great to be able to force a full re-render for these rare situations within the editor / plugin framework.

I have a similar issue -- in my application, disparate text regions can be highlighted and grouped together (stored as separate slate Marks, sharing a highlightId). When I hover over one region, I'd like to visually highlight all of its related regions (those sharing the same highlightId. Ideally, I'd be able to do this in renderMark by looking at editor.value.data.get('highlightedIds'), but am running into the same no-update issue.

I suppose one alternate approach is to make renderMark render a connected component (I'm using redux) and then it'll update. I'll also look into the new React context API.

Here's my original code for doing a force refresh but I have a newer version posted below but which I haven't posted (just wrote right now):

export default function ForceRefreshPlugin() {
  let forceRefresh = false
  return {
    // This method called directly onto the Object.
    // 
    // const forceRefreshPlugin = ForceRefreshPlugin()
    // plugins = [..., forceRefreshPlugin]
    // forceRefreshPlugin.refresh()
    refresh() {
      forceRefresh = true     
    },
    // Cleared after the editor renders once
    componentDidUpdate() {
      forceRefresh = false
    },
    // This code always returns true (forcing the refresh) until the
    // entire Editor is redrawn at which point `componentDidUpdate` gets
    // called and then this returns null which reverts to normal refresh
    // rules.
    shouldNodeComponentUpdate(props, nextProps) {
      return forceRefresh ? true : null
    },
  }
}

The following should work though and uses a command instead.

export default function ForceRefreshPlugin() {
  let forceRefresh = false
  return {
    // Cleared after the editor renders once
    componentDidUpdate() {
      forceRefresh = false
    },
    // This code always returns true (forcing the refresh) until the
    // entire Editor is redrawn at which point `componentDidUpdate` gets
    // called and then this returns null which reverts to normal refresh
    // rules.
    shouldNodeComponentUpdate(props, nextProps) {
      return forceRefresh ? true : null
    },
    commands: {
      refresh() {
        forceRefresh = true
      }
    }
  }
}

You should be able to refresh then using this code:

editor.refresh()

If anyone comes late to the party as I did, I found modern React's useContext() hook works great for this. Basically take the approach @ianstormtaylor suggested but use the built-in hooks instead of react-broadcast (React's contexts are based on react-broadcast anyway IIRC). Somewhere up above the editor provide a value for this context and use the hook in your component that's used from renderMark(), etc.

  1. Why is this closed? Isn't it rather common that I want to update the editor manually?

  2. Me beeing even later to the Party: @ksimons Could you elaborate a little on this? I don't exactly understand.

@Obiwarn I just mean that if you have some context up your react tree, you can call useContext() somewhere in one of your components to get that context. Setting that context to a new value will force any component that calls useContext() to re-render.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gorillatron picture gorillatron  Â·  3Comments

chrpeter picture chrpeter  Â·  3Comments

bunterWolf picture bunterWolf  Â·  3Comments

adrianclay picture adrianclay  Â·  3Comments

chriserickson picture chriserickson  Â·  3Comments