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

fix(ui): properly reflects hook changes in ui #10268

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jmikrut
Copy link
Member

@jmikrut jmikrut commented Dec 30, 2024

Fixes #9882 and #9691

In 2.0, we would accept data coming back from an update operation and then reflect those changes in UI.

However, in 3.0, we did not do that anymore - meaning you could change a document with hooks in beforeChange or afterChange, but then not see the changes made on the server.

Now, we store initialState and initialData in DocumentInfoProvider and update them when we save a document.

Note that this does not apply to autosave.

@jacobsfletch jacobsfletch changed the title fix: properly reflects hook changes in ui fix(ui): properly reflects hook changes in ui Jan 2, 2025
}, [initialState])

useEffect(() => {
setInitialState(initialState)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have the identical useEffect functions written twice here.

@@ -260,6 +266,14 @@ const DocumentInfo: React.FC<
[],
)

useEffect(() => {
setInitialState(initialState)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This useEffect appears unnecessary. Do you mean to put initialStateFromProps as the dependency?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, a useMemo might make more sense.

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

Successfully merging this pull request may close these issues.

Payload requires page refresh on BeforeChange hook
2 participants