Passing custom props to nodes

[SamyPesse]

In our application, some of our node components requires some props from the parent.

The solution we tried was to create a schema depending on these props:

function makeSchema(commonProps) {
    return {
        nodes: {
            paragraph: (props) => (
                <Paragraph {...props} {...commonProps}>
                    {props.children}
                </Paragraph>
            )
        }
    };
}

But Slate doesn't re-render when the schema changed.

So I'm proposing 3 solutions:

1. Adapt the shouldComponentUpdate to re-render when the schema changed
   • Pro: it seems coherent
   • Cons: How to compare prevProps.schema with props.schema ? If the schema is inlined in the render() method, and so unique to each render call
2. Pass all other props of Editor to the nodes
   • Cons: we have to avoid conflicts between prop's names
3. Add a prop props to Editor: <Editor props={{ customPropToPassToNode: 'test' }} ... />

@ianstormtaylor @Soreine what do you guys think ?

[Soreine]

Solution 2. and 3. boil down to passing props to the nodes. When one of these prop change, every node will have to re-render, since Slate cannot know what they are used for.

I always thought Slate would re-render everything on schema change. Solution 1. would break people's code. I would not try to compare schemas but using referential equality, it seems bug-prone.

[bunterWolf]

I'm not sure if this is a solid way to handle that, but it works well for me right now.
I can access all the editor props in the schema by props.editor.props

<Editor
  state={this.props.editorState}
  onChange={this.onChange}

  yourProp="yourProp"
/>

schema = props => {
  props.editor.props.yourProp
}

[SamyPesse]

@bunterWolf Yes it works for the first rendering. But if your prop yourProp changes, it'll not re-render the nodes.

[SamyPesse]

I've started a PR: #764

[ianstormtaylor]

Hey @SamyPesse interesting use case!

What is the exact use case? What's props do the nodes need from the editor? Curious to get my mind around it.

I've got a few ideas for solutions but not sure I know the exact problem enough to know which one feels right.

[SamyPesse]

@ianstormtaylor I've started describing the use-case in the PR (#764 (comment)), but here are more details:

We want to highlight (and change the UX) of some nodes according to the browser's url. Basically the user can click on a comment icon on the right, it changes the url to comments/<node.key>.

We are using react-router, and we want the nodes to know what is the value of the param in the url. The "selected" node is highlighted.

We first thought about using react-router's withRouter which uses the context, but it doesn't work since Slate has some shouldComponentUpdate which prevent the deep nodes to be re-render (remix-run/react-router#5037).

This is because context updates do not trigger re-render (facebook/react#2517 (comment)), basically a React's context should be constant or provide its own subscribing model.

We also have other use cases for this, where our node components need to know about props from the editor's parent.

[ianstormtaylor]

@SamyPesse Okay, damn, that is hard haha. What about if we allowed plugins to listen to the lifecycle events of the editor, such that you could do something like this inside a plugin:

function plugin() {
  const store = new Store()  

  function editorDidMount(editor, props) {
    store.set(props.pathname)
  }

  function editorDidUpdate(editor, props) {
    store.set(props.pathname)
  }

  class Comment extends React.Component {

    state = {
      pathname = store.get(),
    }

    componentDidMount = () => {
      store.on(this.onPathnameChange)
    }

    componentWillUnmount = () => {
      store.off(this.onPathnameChange)
    }

    onPathnameChange = (pathname) => {
      this.setState({ pathname })
    }

    render = () => {
      const { pathname } = this.state
      const { node } = this.props
      const id = node.data.get('id')
      const active = pathname == id
      return ...
    }

  }

  const schema = {
    marks: {
      comment: Comment,
    }
  }

  return {
    editorDidMount,
    editorDidUpdate,
    schema,
  }
}

I think that would allow you to get a lot fewer updates, since you'd be able to only update when the props above changed, or better yet only when their impacts below changed. But it's just a quick idea, could be some gaping holes.

Not the most amazing, since you're back to monitoring a store with handlers in a not very React way. But not sure there's a better way unless React supported this natively with context in a way that handled updates.

[oyeanuj]

@ianstormtaylor +1 to the idea of plugin listening to lifecycle events. This idea of yours also seems to touch upon the conversation in #715 as well.

I'd also like to throw in another related usecase that I had mentioned in #764 to @SamyPesse - communicating from the editor to specific nodes.

I feel like all three requests ('global context', 'props from editor to plugin', and the one I just mentioned above) seem to be getting around limitations in current patterns of passing information via the editor and the discussion might help from keeping all of them in mind.

[ianstormtaylor]

@oyeanuj thanks, I agree with you, keeping those all in mind at once sounds good. I think for the spellchecking case of #764, you might actually be able to handle that in the node itself? Instead of needing to pass them down from the editor?

@SamyPesse Actually, react-router provides a withRouter HOC utility that gives you access to the router in any subcomponent. Maybe the comments (or whatever component wants to use it) should just use that subcomponent and that will get the re-renders happening naturally?

Edit:

If you are using withRouter to prevent updates from being blocked by shouldComponentUpdate, it is important that withRouter wraps the component that implements shouldComponentUpate.

Nevermind about withRouter.

[oyeanuj]

I think for the spellchecking case of #764, you might actually be able to handle that in the node itself?

@ianstormtaylor I was considering handling it in the node for fetching URL metadata, and possibly even for mentions (For spell/grammar check, I was going to send the whole document for some other analysis as well, so just the node wouldn't work).

But the only downside to that method is that the rendering components can't be 'dumb/presentational' anymore and need to be connected to outside. A plugin user can't just supply a component to handle the rendering.

An alternative would be something similar to @SamyPesse' original comment or @bunterWolf's suggestion above which circles back to this ticket 😄

[hueypeard]

I'm in the same boat. If it helps, my use case is a question editor that has void nodes that show the default answers of user-facing inputs inline (e.g. choice, text). They (the default answers) are stored in an object elsewhere and passed in as a prop to slate.

[SamyPesse]

Finally this is not required, and it should not be done by slate.

The solution is to use a store like @ianstormtaylor suggested but in the parent component in your apps (create the store, create a schema from this store, update the store when the props of your component changes).

It works very well in my application. Closing this issue.

[ianstormtaylor]

Awesome, thanks @SamyPesse!

[oyeanuj]

@ianstormtaylor I'd still vote a first-class pattern or API to address the all these related cases, like we discussed above. Since this ticket is closed, should we address those discussions in their related tickets or open an umbrella ticket which deals with these cases (#709, #715, and this one)?

@SamyPesse In your solution, by 'parent component', are you referring to the editor or its parent or something else?

[ianstormtaylor]

Samy meant the parent of the editor.

I'm still down for the one that adds a data property to the state model itself. And then that could easily be used as the store in a way to keep the data in a serialization location.

I think I might be down to still have the lifecycle methods as plugin methods. Although I'd like to experiment with making plugins HOCs to achieve it first instead.

[oyeanuj]

@ianstormtaylor Thanks for the clarification. So, in this method, all the nodes would get re-rendered.. wondering if you have ideas of how best to address where one only wants to communicate to specific nodes from the editor level (without needing every other node to re-render) in my usecase?

[ianstormtaylor]

@oyeanuj I think for that case you'd want to setup your own store/subscribing pattern like I assume what Samy's done, and use that to then call setState inside the node. That way only that single node will change when the event happens. Does that make sense?

I'm just not sure Slate should standardize it yet, unless we come across a lot of use cases that are always the exact same. It feels like the subscription handling stuff could get very complex.

[oyeanuj]

@ianstormtaylor Fair enough!