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: Invalid shared value access displaying when code is correct #6631

Merged
merged 7 commits into from
Oct 29, 2024

Conversation

MatiPl01
Copy link
Member

Summary

The problem was caused by our useDerivedValue hook which set ref to null in the useEffect cleanup method.

useEffect(() => {
  return () => {
    initRef.current = null;
  };
}, []);

The warning was shown during hot reload, because react calls cleanup functions in hooks, thus the initRef was reset in the useDerivedValue hook, which resulted in execution of this code that should run only when the component mounts for the first time:

if (initRef.current === null) {
  initRef.current = makeMutable(initialUpdaterRun(updater));
}

Because of that, initialUpdaterRun executed the updater callback on the JS thread during hot reload, while the component was rendered, which resulted in reading shared value's .value property during the render phase.

@MatiPl01 MatiPl01 self-assigned this Oct 25, 2024
@MatiPl01 MatiPl01 requested review from tomekzaw and tjzel October 25, 2024 09:18
@MatiPl01 MatiPl01 requested a review from tjzel October 29, 2024 14:39
Copy link
Collaborator

@tjzel tjzel left a comment

Choose a reason for hiding this comment

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

Let's go!!!

@MatiPl01 MatiPl01 added this pull request to the merge queue Oct 29, 2024
Merged via the queue into main with commit 54261ba Oct 29, 2024
9 checks passed
@MatiPl01 MatiPl01 deleted the @matipl01/fix-invalid-shared-value-access-warning branch October 29, 2024 14:42
tjzel pushed a commit that referenced this pull request Nov 25, 2024
## Summary

The problem was caused by our `useDerivedValue` hook which set ref to
`null` in the `useEffect` cleanup method.

```tsx
useEffect(() => {
  return () => {
    initRef.current = null;
  };
}, []);
```

The warning was shown during hot reload, because react calls cleanup
functions in hooks, thus the `initRef` was reset in the
`useDerivedValue` hook, which resulted in execution of this code that
should run only when the component mounts for the first time:

```tsx
if (initRef.current === null) {
  initRef.current = makeMutable(initialUpdaterRun(updater));
}
```

Because of that, `initialUpdaterRun` executed the `updater` callback on
the JS thread during hot reload, while the component was rendered, which
resulted in reading shared value's `.value` property during the render
phase.
@tjzel tjzel mentioned this pull request Nov 25, 2024
tjzel pushed a commit that referenced this pull request Nov 26, 2024
## Summary

The problem was caused by our `useDerivedValue` hook which set ref to
`null` in the `useEffect` cleanup method.

```tsx
useEffect(() => {
  return () => {
    initRef.current = null;
  };
}, []);
```

The warning was shown during hot reload, because react calls cleanup
functions in hooks, thus the `initRef` was reset in the
`useDerivedValue` hook, which resulted in execution of this code that
should run only when the component mounts for the first time:

```tsx
if (initRef.current === null) {
  initRef.current = makeMutable(initialUpdaterRun(updater));
}
```

Because of that, `initialUpdaterRun` executed the `updater` callback on
the JS thread during hot reload, while the component was rendered, which
resulted in reading shared value's `.value` property during the render
phase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants