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

Slate docs say to use useMemo, should be useRef #3198

Closed
jas7457 opened this issue Dec 2, 2019 · 1 comment · Fixed by #4766
Closed

Slate docs say to use useMemo, should be useRef #3198

jas7457 opened this issue Dec 2, 2019 · 1 comment · Fixed by #4766

Comments

@jas7457
Copy link

jas7457 commented Dec 2, 2019

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

Bug

What's the current behavior?

In the docs, it says to use useMemo for the editor instance passed to the <Slate> component so it is "stable across renders". useMemo does not guarantee identity between renders (https://reactjs.org/docs/hooks-reference.html#usememo) and talking with @ianstormtaylor, it sounds like identity is needed. between renders.

Slate: 0.50.7

What's the expected behavior?

The docs should be updated to use a ref (or some other stable memo besides useMemo/useCallback/etc). Slate itself could memoize the value passed to it if identity is necessary, or inversion of control could be maintained for the user of Slate to pass a memoized version.

Note:

A simple hook that I wrote can help to initialize a ref if it's expensive to compute. It works similar to useState's lazy initialization, but there wasn't a way to lazily set a ref.

import {useRef, MutableRefObject} from 'react';

// using an object here instead of something like "null" or "undefined" or "Infinity"
// that way we truly know if we need to call the init function or not because the user of this can never pass something that === initialValue
const initialValue = {};

/**
 * Allows for lazy refs that only call the callback once
 * This also helps guarantee values between renders, unlike useMemo or useCallback which makes no such guarantee
 */
export default function useLazyRef<T>( init:() => T ):MutableRefObject<T> {
	const ref = useRef<T | typeof initialValue>( initialValue );

	if ( ref.current === initialValue ) {
		ref.current = init();
	}

	return ref as MutableRefObject<T>;
}
@ianstormtaylor
Copy link
Owner

Seems like we can use https://github.com/alexreardon/use-memo-one and maybe expose it as a useStable hook for people. Since it does seem to be asking a lot if people have to figure out the stability for themselves.

dmnd added a commit to dmnd/slate that referenced this issue Jan 5, 2022
As already discussed almost a year ago in ianstormtaylor#3198, `useMemo` doesn't guarantee persistence. This breaks Fast Refresh.

`useState` without a setter is a straightforward fix that avoids dependencies; let's get the docs updated so Fast Refresh works again.

Fixes ianstormtaylor#3198
dylans pushed a commit that referenced this issue Jan 8, 2022
As already discussed almost a year ago in #3198, `useMemo` doesn't guarantee persistence. This breaks Fast Refresh.

`useState` without a setter is a straightforward fix that avoids dependencies; let's get the docs updated so Fast Refresh works again.

Fixes #3198
DougReeder pushed a commit to DougReeder/slate that referenced this issue Apr 3, 2022
As already discussed almost a year ago in ianstormtaylor#3198, `useMemo` doesn't guarantee persistence. This breaks Fast Refresh.

`useState` without a setter is a straightforward fix that avoids dependencies; let's get the docs updated so Fast Refresh works again.

Fixes ianstormtaylor#3198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants