Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: facebook/react
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 603e6108f39c6663ec703eed34a89ff1bf0cb70c
Choose a base ref
...
head repository: facebook/react
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: b7e21579220042c0a60179e2f40f121684e637eb
Choose a head ref
  • 2 commits
  • 4 files changed
  • 3 contributors

Commits on Oct 31, 2024

  1. Fix Ref Lifecycles in Hidden Subtrees (#31379) 

    ## Summary

We're seeing certain situations in React Native development where ref
callbacks in `<Activity mode="hidden">` are sometimes invoked exactly
once with `null` without ever being called with a "current" value.

This violates the contract for refs because refs are expected to always
attach before detach (and to always eventually detach after attach).
This is *particularly* bad for refs that return cleanup functions,
because refs that return cleanup functions expect to never be invoked
with `null`. This bug causes such refs to be invoked with `null`
(because since `safelyAttachRef` was never called, `safelyDetachRef`
thinks the ref does not return a cleanup function and invokes it with
`null`).

This fix makes use of `offscreenSubtreeWasHidden` in
`commitDeletionEffectsOnFiber`, similar to how
ec52a56
did this for `commitDeletionEffectsOnFiber`.

## How did you test this change?

We were able to isolate the repro steps to isolate the React Native
experimental changes. However, the repro steps depend on Fast Refresh.

```
function callbackRef(current) {
  // Called once with `current` as null, upon triggering Fast Refresh.
}

<Activity mode="hidden">
  <View ref={callbackRef} />;
</Activity>
```

Ideally, we would have a unit test that verifies this behavior without
Fast Refresh. (We have evidence that this bug occurs without Fast
Refresh in real product implementations. However, we have not
successfully deduced the root cause, yet.)

This PR currently includes a unit test that reproduces the Fast Refresh
scenario, which is also demonstrated in this CodeSandbox:
https://codesandbox.io/p/sandbox/hungry-darkness-33wxy7

Verified unit tests pass:

```
$ yarn
$ yarn test
# Run with `-r=www-classic` for `enableScopeAPI` tests.
$ yarn test -r=www-classic
```

Verified on the internal React Native development branch that the bug no
longer repros.

---------

Co-authored-by: Rick Hanlon <[email protected]>
    @yungsters @rickhanlonii
    yungsters and rickhanlonii authored Oct 31, 2024
    Configuration menu
    Copy the full SHA
    ea3ac58 View commit details
    Browse the repository at this point in the history

  2. [Flight] Handle errors during JSON stringify of console values (#31391) 

    When we serialize debug info we should never error even though we don't
currently support everything being serialized. Since it's non-essential
dev information.

We already handle errors in the replacer but not when errors happen in
the JSON algorithm itself - such as cyclic errors.

We should ideally support cyclic objects but regardless we should
gracefully handle the errors.
    @sebmarkbage
    sebmarkbage authored Oct 31, 2024
    Configuration menu
    Copy the full SHA
    b7e2157 View commit details
    Browse the repository at this point in the history
Loading