Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Cannot update slate state externally #4612

Open
Ghost---Shadow opened this issue Oct 20, 2021 · 39 comments
Open

Bug: Cannot update slate state externally #4612

Ghost---Shadow opened this issue Oct 20, 2021 · 39 comments
Labels

Comments

@Ghost---Shadow
Copy link

Description

The PR #4540 removed the ability to update the slate state using the value prop. As a result, we cannot inject externally changed state anymore.

If there is another way to update the state externally, then the documentation should be updated as well.
https://docs.slatejs.org/walkthroughs/06-saving-to-a-database

Environment

  • Slate Version: "^0.67.0"
  • Operating System: Windows
  • Browser: Firefox
@circlingthesun
Copy link
Contributor

Ouch, we got bitten hard by this. Downgraded for now.

@circlingthesun
Copy link
Contributor

@Ghost---Shadow if you look at what was changed (https://github.com/ianstormtaylor/slate/pull/4540/files), you can easily get the same behaviour by setting a key property on the Slate element and incrementing whenever your state changes.

@aboveyunhai
Copy link

@Ghost---Shadow if you look at what was changed (https://github.com/ianstormtaylor/slate/pull/4540/files), you can easily get the same behaviour by setting a key property on the Slate element and incrementing whenever your state changes.

Can you elaborate a little bit more with the key hook code sample. Wouldn't it cause some re-render issues of entire slate component while changing the value or visually the text.
I'm ok with the change but little bit lose the direction to achieve the similar behavior in a proper way. Just roll back to last version until the doc fixed,

I am very aware of any slate "breaking" change but it's a still a powerful blackbox machine to me, still on basic/user level and try to understand the internal logic.

@ivan-nemtinov
Copy link

ivan-nemtinov commented Nov 1, 2021

A temporary solution: update editor.children instead of value, as specified in [email protected] release notes.

@Ghost---Shadow
Copy link
Author

This is a hack, not a solution. If this is the official way of updating the state, then the documentation should be updated as well.

@ivan-nemtinov
Copy link

It works, but I agree, it's a hack. Setting editor.children directly also requires manual fixing of selection, because selected range remains the same even if some new nodes are added above the selection.

@aboveyunhai
Copy link

aboveyunhai commented Nov 3, 2021

It works, but I agree, it's a hack. Setting editor.children directly also requires manual fixing of selection, because selected range remains the same even if some new nodes are added above the selection.

It does not seem like a hack based on the PR discussion. It's just not a React way, and not necessary to be a React way.
but selection is definitively a problem. And it's confusing comparing to the past implementation.

@jan-martinek
Copy link

jan-martinek commented Nov 10, 2021

It took me several days to find out that changes in #3216 were silently reversed in #4540. It would be great to reflect the changes not only in docs as others say above, but also in API (eg revert value to defaultValue or initialValue) as the value prop implies controlled behavior.

It‘s possible that out there are many others struggling to find out what went wrong (in our case it wasn’t an upgrade going bad — that would make me go for changelists sooner, but documentation being out of date and API pretending that everything is okay).

@jan-martinek
Copy link

(Hence, I’d propose relabeling this as an improvement and changing the name of the prop — such change would highlight its breaking nature, as the difference between un- and controlled components is night and day.)

@Pines-Cheng
Copy link

Pines-Cheng commented Dec 10, 2021

A solution: update editor.children instead of value, as specified in [email protected] release notes.

It's really not a react way.

@ivan-nemtinov
Copy link

It's really not a react way.

Of course it's not. I hope somebody will make a refactoring.

@aboveyunhai
Copy link

aboveyunhai commented Dec 10, 2021

It's really not a react way.

Of course it's not. I hope somebody will make a refactoring.

slate was claimed to be built on top of React in github main page, but we have slate react dedicated for React logic purpose if I'm not mistaken with the major functionalities. So does the same claim applies on slate core(slate)?

Now the problem is that if this PR #4540 is merged intentionally, does it mean the slate core actually does not rely on react?
which is extremely important for the future release.
as well as my questionable past statement .

It's just not a React way, and not necessary to be a React way. ?

Strictly speaking: controlled component vs uncontrolled component instead of react way.
Indeed, there are some silent disagreements contributors were unware of.
I guess it's good to ask @bryanph for clarification and his plan.

@ivan-nemtinov
Copy link

slate was claimed to be built on top of React in github main page, but we have slate react dedicated for React logic purpose if I'm not mistaken with the major functionalities. So does the same claim applies on slate core(slate)?

Now the problem is that if this PR #4540 is merged intentionally, does it mean the slate core actually does not rely on react? which is extremely important for the future release. as well as my questionable past statement .

Yes, slate core doesn't rely on react by design. slate-react modifies editor.children in react hooks.

Indeed, there are some silent disagreements contributors were unware of. I guess it's good to ask @bryanph for clarification and his plan.

Would be nice if you did it!

@bryanph
Copy link
Contributor

bryanph commented Dec 10, 2021

@aboveyunhai slate was redesigned at some point so that the slate package is just for modifying the document and slate-react is for the react specific part (rendering the changes to the DOM). I think you got it right in the controlled component vs uncontrolled component part. I originally argued in an issue that it should be explicitly an uncontrolled component because some people were getting problems with the selection not updating as a result of setting the "controlled" value state, see #3575 (comment). I elaborated on this here: #4540 (comment).

Note that an Editor.reset method was proposed there which would then also handle the selection resetting and the history resetting (which wasn't handled by passing a different value prop). Will see if I can find some time in the next few weeks to give that a shot (or someone else can give it a shot of course 😄). As mentioned above setting editor.children explicitly to an initial value is the recommended way for now. I agree that's not the most elegant solution but hopefully an Editor.reset API of some form will resolve that.

@mlajtos
Copy link

mlajtos commented Feb 2, 2022

Changing content via Transform.transform(op) does not reflect changes visually, however the editor.children does change. Reproduction code: https://codesandbox.io/s/great-waterfall-sp2jm?file=/src/App.tsx

@bryanph What to do in this case?

@bryanph
Copy link
Contributor

bryanph commented Feb 2, 2022

@mlajtos use editor.apply instead

@odusseys
Copy link

odusseys commented Feb 3, 2022

We are building a collaborative editing tool and this change is pretty much a killed when it comes to reconciling state from the server. Using a key to fully re-render is destructive and pretty much kills the UX as all selection state is lost.
I would strongly consider not exposing an uncontrolled API as this makes it very tough to use properly for any non-basic use case, especially when external state can override application state (which is pretty much all the time in 2022).

@odusseys
Copy link

odusseys commented Feb 3, 2022

@bryanph do you have an code snippet to explain how to do this ? the documentation for operations is laconic

@jmikrut
Copy link

jmikrut commented Feb 8, 2022

This just caused me to blow about 3 million brain cells. We use Slate extensively in Payload and also need to programmatically handle incoming values. For now, I've done as described above in this thread and added a key based on initialValue to a div containing the Slate editor.

Here is the commit for anyone interested. Obviously Payload's implementation is quite complex but the actual changes here are fairly simple. This works for now but man that threw me for a loop.

@spudly
Copy link

spudly commented Feb 16, 2022

For those who just need a quick workaround, here's how I fixed it in my code:

Custom useForceUpdate hook defined elsewhere:

const useForceUpdate = () => {
  const [, setState] = useState<number>(0);

  const forceUpdate = useCallback(() => {
    setState(n => n + 1);
  }, []);

  return forceUpdate;
};

Code inside my component that renders <Slate>:

  const forceUpdate = useForceUpdate();
  useEffect(() => {
    editor.children = value;
    forceUpdate();
  }, [editor, value, forceUpdate]);

@franky47
Copy link

I stumbled upon this issue when connecting the Slate value to a form (using Formik), and trying to reset it on submit.

Here's my "hack", which updates the key only when the value changes to the default. It's not a fully-controlled solution, but it did the trick:

const defaultRichTextValue: Descendant[] = [
  {
    type: 'paragraph',
    children: [{ text: '' }],
  },
]

function useResetKey(value: Descendant[]) {
  const [key, setKey] = React.useState(0)
  React.useEffect(() => {
    if (JSON.stringify(value) === JSON.stringify(defaultRichTextValue)) {
      setKey(n => n + 1)
    }
  }, [value])
  return key
}

const RichTextEditor = ({
  value,
  onChange,
  ...props
}) => {
  const resetKey = useResetKey(value)
  const editor = React.useRef(withHistory(withReact(createEditor())))
  return (
    <Slate
      editor={editor.current}
      value={value}
      onChange={onChange}
      key={resetKey}
    >
      <Editable {...props} />
    </Slate>
  )
}

@ArthurBZhou
Copy link

ArthurBZhou commented Apr 22, 2022

thx! @franky47 Hope this bug will be ruled out as soon as possible!

@jmadelaine
Copy link

jmadelaine commented Jun 18, 2022

We are also seeing this as a major blocker to using Slate. We have multiple ways to display comment threads (sidebar and a floating comment overlay), and sometimes both are visible when typing a new comment, meaning they share state. It's pretty fundamental to have the ability to control an input's value externally, and in fact, one way data flow is one of the core pillars of React. This is a bug, and the key hack is just that, a hack.

@jonathan-chin
Copy link

this is also a dealbreaker to adoption for me.

@gsvh
Copy link

gsvh commented Dec 13, 2022

Really loved using Slate until I faced this problem and read this thread. I have been stuck on this for a while, and the solutions mentioned here are way below the standard set by how the rest of Slate works.

@gsvh
Copy link

gsvh commented Dec 13, 2022

The solution I used doesn't need any useEffects or extra states

<Slate editor={editor} value={value} key={JSON.stringify(value)}>

I stringify the value and use that as the key, so that when the value changes, the key changes and the component is rerendered

@Rinzyy
Copy link

Rinzyy commented Jan 12, 2023

I've been finding various options to resolve this issue, and it appears that your solution is the simplest and most efficient method. This should be on the documentation.

@hypeJunction
Copy link

Using keys and forcing the component to re-render is a no go: major performance hog and prone to errors with caret position and selection.
What are our options here? Any intention to fix this issue, and if so, any particular timeline in sight? I love Slate so far, but this particular issue is making me reconsider.
What legacy version can I roll back to to get controlled react components? Also, what's up with versioning here? Why is everything on 0.x.x if there were many breaking changes already?
I would be willing to invest time to figure out what's going on in the source code and how we can make things controlled, but I need some indication if someone is already working on it, and if it's something that the core intends to support again - otherwise I will just fork and go.

@macrozone
Copy link
Contributor

I am maintainin https://github.com/react-page/react-page which uses slate for its rich-text editor. Since the infamous change to an uncontrolled component, we suffer from bugs and inconsistencies. Setting the value from outside is important for us, because we have our own undo/redo mechanism.

Uncontrolled components are in my opinion an antipattern in most cases and the amount of headache it created alone in this thread here shows why. I recently created another issue here #5281 but i think it a duplicate of this here.

I tried different workarounds, but I found none that works consistently. Slate really has to revert this decision, I don't think there is another way.

@hypeJunction
Copy link

What we ended up doing to dynamically modify the state of slate, is passing the ref of the editor to the parent components via a Context wrapper, and using Slate API to mutate nodes. Not the most elegant solution, but it did the trick for us. Though, I would really prefer having a controlled input rather than keeping track of editor refs.

@ggrantrowberry
Copy link

@hypeJunction Do you have any examples of how you did that that you can share by any chance? I'm trying to control undo and redo from outside of Slate and am running into issues.

@hypeJunction
Copy link

hypeJunction commented Mar 13, 2023

@ggrantrowberry Here are some snippets:

Context and utility components defined as such:

import React, { PropsWithChildren, useContext, useEffect, useRef } from 'react'
import { useSlate } from 'slate-react'
import { Editor } from 'slate'

type TemplatedFormContextValue = {
  editors: Map<string, Editor>
}

const TemplatedFormContext = React.createContext<TemplatedFormContextValue | null>(null)

export const TemplatedForm = ({ children }: PropsWithChildren<any>) => {
  const ref = useRef<Map<string, Editor>>()

  if (!ref.current) {
    ref.current = new Map()
  }

  return (
    <TemplatedFormContext.Provider
      value={{
        editors: ref.current
      }}
    >
      {children}
    </TemplatedFormContext.Provider>
  )
}

export const TemplatedFormEditorReference = ({ name }: { name: string }) => {
  const editor = useSlate()
  const ctx = useTemplatedForm()

  useEffect(() => {
    if (ctx?.editors) {
      ctx.editors.set(name, editor)
    }
  }, [editor, name])

  return null
}

export const useTemplatedForm = () => {
  return useContext(TemplatedFormContext)
}

You then add the reference to the context when rendering Slate

export const RichEditor = forwardRef(
  (
    { value, onChange, before, toolbar, after, name, ...rest }: RichEditorProps,
    ref: ForwardedRef<HTMLDivElement>
  ) => {
    const { editor } = useSlateEditor()

    const defaultValue = Array.isArray(value) && value.length > 0 ? value : defaultEditorValue

    return (
      <InputBox
        variant='outlined'
        color='neutral'
        control={
          <Slate value={defaultValue} editor={editor} onChange={onChange}>
            {before}
            <div className={b()}>
              {toolbar}
              {name ? <TemplatedFormEditorReference name={name} /> : null}
              <EditableArea ref={ref} {...rest} />
            </div>
            {after}
          </Slate>
        }
      />
    )
  }
)

And to modify the contents of the editor, you would use slate apis:

import { Transforms } from 'slate'
import { cloneDeep } from 'lodash'

export const TemplatePicker = ({ templates }: InsertTemplateProps) => {
  const ctx = useTemplatedForm()

  const appendTemplate = (fields: { name: string; text: StructuredText[] }[]) => {
    fields.forEach((field) => {
      const editor = ctx?.editors.get(field.name)
      if (editor) {
        editor.children.map((item) => {
          Transforms.delete(editor, { at: [0] })
        })
        field.text.map((node) => {
          Transforms.insertNodes(editor, cloneDeep(node))
        })
      }
    })
  }

   // template selector with callback
}

This could probably be optimized. The code was written by our junior with some initial input from me, and I haven't seen the latest version.

@Neczesk
Copy link

Neczesk commented Jun 21, 2023

I just wanted to chime in, I really wanted to use slate for my project but it became unusable to me because of the lack of a simple way to update the state externally. If I had the time I'd try to come up with a pull request and change it myself, but I don't have the knowledge to do it in a reasonable time frame.

@joulev
Copy link

joulev commented Jul 18, 2023

Just to add in, I have to make my editor controllable as well, so I did something like this, using the key idea as described above, but only update the key when the change comes from external sources.

"use client";

import { useMemo, useState } from "react";
import { Descendant, createEditor } from "slate";
import { Editable, Slate, withReact } from "slate-react";

const initialValue: Descendant[] = [
  { type: "paragraph", properties: {}, children: [{ text: "Hello World!" }] },
];

function Editor({
  value,
  onChange,
}: {
  value: Descendant[];
  onChange: (value: Descendant[]) => void;
}) {
  const editor = useMemo(() => withReact(createEditor()), []);
  return (
    <Slate editor={editor} initialValue={value} onChange={onChange}>
      <Editable />
    </Slate>
  );
}

export default function Page() {
  const [value, setValueRaw] = useState(initialValue);
  const [key, setKey] = useState(0);
  function setValue(value: Descendant[]) {
    setValueRaw(value);
    setKey(key + 1);
  }
  return (
    <div>
      <button type="button" onClick={() => setValue(initialValue)}>
        Reset
      </button>
      <Editor key={key} value={value} onChange={setValueRaw} />
    </div>
  );
}

and I just use setValue anywhere I need to update the state outside Slate.

Can't say rerendering the entire editor is good for performance, but this is the only solution applicable to our app; assigning to editor.children looks too un-Reacty and sometimes causes errors.

@aboveyunhai
Copy link

aboveyunhai commented Jul 18, 2023

@joulev as I came through a lot of uncontrolled components (canvas, dnd, table editing, occasionally input), I hit the limitation of React way on doing render and state control, I sincerely believe Slate (or other text-editors) should belong to the uncontrolled component instead of controlled component due to the optimization, the granular state update, potentially collaborative mode, and a lot of other things that you could only be done via uncontrolled component.

To be more aggressive, doing the React way here is not the correct way of doing React way. 😂
The related original PR (everyone here blamed) that removed the external state binding, finally clicks for me.

Uncontrolled component as well as mutability really sounds like a negative term in the React world, but in fact, we just fallback to the original de facto JavaScript way of handling the DOM, especially for something complex like text-editor. Since this issue continuously and occasionally got picked up by someone. I highly recommended people who read and agree my post, to migrate their implementation to uncontrolled component. If I had time to pick up the Slate code I had written, I will simply change everything based on what I said here.

@jlarrubiaq
Copy link

Same problem here, we tried to update from an old version of Slate so we could benefit from the use of plain objects instead on Inmutable but this is preventing us from completing the migration. We might need to look for alternatives if Slate doesn't provide a non-hacky way to control the component value. The solutions above break the editor selection when externally changing the value.

@seyhak
Copy link

seyhak commented Oct 26, 2023

the fix is in documentation, I found it after few hours ...
If you want to update the editor's content in response to events from outside of Slate, you need to change the children property directly. The simplest way is to replace the value of editor.children editor.children = newValue and trigger a re-rendering (e.g. by calling editor.onChange() in the example above). Alternatively, you can use Slate's internal operations to transform the value, for example:
https://docs.slatejs.org/walkthroughs/06-saving-to-a-database

@cheestudio
Copy link

cheestudio commented Oct 29, 2023

@seyhak Cannot thank you enough! It's so obvious and simple, yet I was overcomplicating it. It's clear in the docs, but it's also apparently really easy to miss! In my use case, replacing the values and triggering a re-render was two lines and perfect for what I needed.

@Kevo-gmm
Copy link

Kevo-gmm commented May 8, 2024

@Ghost---Shadow if you look at what was changed (https://github.com/ianstormtaylor/slate/pull/4540/files), you can easily get the same behaviour by setting a key property on the Slate element and incrementing whenever your state changes.

works perfectly fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests