ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.81k stars 3.24k forks source link

Modification to onChange cannot get latest state #3621

Open jolanglinais opened 4 years ago

jolanglinais commented 4 years ago

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

Bug 🐛

What's the current behavior?

If I have a state which should update for each onChange() call on the editor, the onChange() only has access to the initial state. It also somehow reverts to the barebones editor state during this behavior.

Cannot find a descendant at path [0] in node: 
{
  children: [],
  operations: [],
  selection: null,
  marks: null,
  history: {
    undos: [],
    redos: []
  }
}
GIF of issue in action

Apr-17-2020 15-35-34

Sandbox of issue reproduction

https://codesandbox.io/s/slate-repo-irmerk-17-04-2020-mebkd

Information about your OS, browser, Slate version, etc.

Slate: 0.57.1 Browser: Chrome OS: Mac

What's the expected behavior?

Slate should handle updating of the editor without losing its current value, selection, etc when one of the custom high order functions wrapping the creation of the editor changes

cymoo commented 4 years ago

Slate's state should be fully controlled by itself; otherwise view may not be rendered correctly. It has some similarity to React's virtual dom. If you modify DOM's structure using some APIs like el.setAttribute, el.innerHtml = 'xxx', React will be baffled or ignore the modification.

jolanglinais commented 4 years ago

Right. But this is the state of App, the container to Slate. If the state from above Slate is needed to be updated within a higher order function placed on the editor, it currently cannot be done.

cameracker commented 4 years ago

Is the issue caused by the useMemo call being rerun too frequently? This is causing the editor to be regenerated everytime the callback is recalculated, which is every onChange. 🤔 This seems preventable but I don't have anything off the top of my head to specifically recommend.

jolanglinais commented 4 years ago

Yes I believe this stems from useMemo, but I think putting the dependency in the useMemo results in the editor rebuilding with it's base state.

My current workaround is putting this state in redux and allowing the HOF on the editor to call the store.getState() function to obtain the current state.

cameracker commented 4 years ago

Looking more at the code sandbox, I'm getting the sense there's a better way to go about this.

Even if we get the count plugin to not regenerate the editor and not break, there's still the issue where the count never makes it into the value because it only gets interpolated in the initial state. A transformation is still required to get it there.

What's the overall goal you're trying to achieve? There might be a better way to go about this

jolanglinais commented 4 years ago

Yeah that sounds about right.

The actual goal is parsing a chunk of text against a template - the templates are asynchronously fetched and placed on the component housing the Slate editor. There is a parse function which we want to happen for any onChange which will identify the chunk of text the user is in, grab it's template, and parse.

We get the parse function into the onChange through a HOF wrapper. As you say, it only gets interpolated on the initial state of the templates, which is empty because they have yet to be fetched.

jonathaneckman commented 4 years ago

I am seeing this same behavior whenever my app hot module reloads. In my case the steps to reproduce are:

  1. Begin typing in an editor.
  2. Make a change in code and save to trigger hot module reload.
  3. After a successful bundle rebuild, the page attempts to refresh and the following error is thrown: Cannot find a descendant at path [0] in node: {"children":[],"operations":[],"selection":null,"marks":null}.

This is confusing since I initialize the editor with a default value of [{ type: 'paragraph', children: [{ text: '' }] }], so where is the editor getting the value reported in the error message?

balintbrews commented 3 years ago

I'm experiencing the same as @jonathaneckman. Copy-pasting the plain text example into a clean CRA project reproduces the issue with hot reloading.

martonlanga commented 3 years ago

Having the same issues using Snowpack hot reload. Did you find a fix for this @jonathaneckman, @balintk?

Update: disabled hot module replacement for that module, seems to have fixed it:

if (import.meta.hot) {
  import.meta.hot.decline()
}

I think CRA has something like // @refresh reset, maybe you could try that @balintk

mikestopcontinues commented 3 years ago

I'm having the HMR problem with NextJS. I can't seem to get the module version of this to work, aka, if (module.hot) { module.hot.decline(); }. Anyone have any ideas? Any other workarounds?

martonlanga commented 3 years ago

@mikestopcontinues You could try importing your module dynamically and disabling SSR

import dynamic from 'next/dynamic'

const DynamicComponentWithNoSSR = dynamic(
  () => import('../components/hello3'),
  { ssr: false }
)
mikestopcontinues commented 3 years ago

@martonlanga , worked like a charm! Thanks!

Dem0n3D commented 3 years ago

How to fix it in CRA-driven project?

mikestopcontinues commented 3 years ago

Has there been any more attention given to this issue? The above fix works well-enough for importing slate components, but trying to develop the actual editor, elements, etc—still a horror. What exactly does hot reload do that makes slate go haywire? Is it just reconstituting the hook values?

CloudDrivenSolutions commented 3 years ago

The resolution with issues relating to CRA was to utilize useRef instead of useMemo.

const editorRef = useRef() if (!editorRef.current) editorRef.current = withReact(createEditor()); const editor = editorRef.current;Ï

jonahallibone commented 2 years ago

@CloudDrivenSolutions This also fixed my issue in NextJS when using next/dynamic did not suffice. I don't know that it's the optimal solution but I will be using it for now. Maybe the maintainer has some idea? @ianstormtaylor

cybro-dev commented 2 years ago

@CloudDrivenSolutions Also solved my issue. Thank you so much!