React: useEffect is broken for React Native with JSC

[bvaughn]

As reported in open source (facebook/react-native/issues/21967), the useEffect hook is broken for React Native when using JavaScriptCore (which affects both iOS and Android).

This is because the setTimeout branch of scheduler specifies a 5000ms delay. This 5000ms is _supposed to be the maximum expiration time_, but in this fork it _ends up being the minimum callback time_ (unless another state update forces us to sync-flush pending callbacks). Since useEffect is passive, this means React won't call it until after a few seconds have passed.

I assume we (React team) haven't noticed this because we're typically using the postMessage implementation, but with JavaScriptCore– React Native ends up using the setTimeout fork.

I think this was broken by commit 4d17c3f (PR #13740) which intentionally changed the delay under the (mistaken) assumption that it would only impact unit tests.

If we want to avoid using setTimeout for React Native, I think we'll need to add a scheduler implementation that does not require postMessage or MessageChannel, since JavaScript Core does not support either.

[bvaughn]

cc @acdlite

[bvaughn]

A repro case for this bug can be found in this Gist. Note that the bug will only repro if you don't connect to a remote debugger (since connecting to Chrome will cause scheduler to use the postMessage API). This bug can also be reproduced in react-dom if you force scheduler to use the setTimeout API.

[brunolemos]

Great that you found the cause!

In addition to this delay, another reported issue is that, when js debug is enabled, the useEffect is one render late:

I didn't wait 5 seconds on this test so it might be related.

[bvaughn]

the useEffect is one render late

I think this is actually the same thing. It would be called after 5 seconds, but you're clicking too soon– in which case, React _synchronously calls it_ before starting on the next render (so it seems to be "late" but really, if you were logging the sequence, you'd see that it's called _before_ the follow up render.)

In other words, I think you're seeing:

1. click
2. render 1
3. click
4. commit 1, render 2

[brunolemos]

Yeah I think so, too (but better confirm)

[bvaughn]

What I described above was what I confirmed when investigating the RN bug report already 😄

If you think you're seeing something different, maybe you could confirm?

[brunolemos]

Now I'm thinking it's not the same thing.

• Like you said, when js debug is enabled it uses the postMessage implementation not the setTimeout one that has the delay bug;
• I just rewrote the code from the gif and waited 5 seconds, but it did not trigger the useEffect after that, with js debug enabled

Try the code with js debug enabled:

import React, { useEffect, useState } from 'react'
import { Button, View } from 'react-native'

export function App() {
  const [count, setCount] = useState(0)

  useEffect(
    () => {
      if (!count) return
      alert(`New value after increment: ${count}`)
    },
    [count],
  )

  return (
    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
      <Button
        onPress={() => setCount(count + 1)}
        title={`Increment (${count})`}
      />
    </View>
  )
}

Expected: Press increment. Should show alert with number N and render N.
Current behavior: Press increment. Show alert with N-1 and render N.

[slorber]

hey, just want to report that the issues I encountered were on Android so both platforms are probably broken

[bvaughn]

Okay. I didn't realize but RN is using JSC on Android as well, so it's not a surprise that it impacts both environments. I've updated the description of this issue.

[jascodes]

@bvaughn Do you think this will be resolved before new year?
I don't wanna start my new project with yuckky.. Classes :laughing:

Thx

[bvaughn]

I hope so! But in the meanwhile, you could probably use useLayoutEffect instead? It shouldn't be affected by this bug since it's called synchronously.

[bvaughn]

react 16.7 has a couple of alphas. The first one depends on scheduler@^0.11.0-alpha but the others all depend on scheduler@^0.12.0-alpha. I'm not sure why this is, but I see that there are some significant changes between the two scheduler alphas– and I feel like back porting my fix to 0.11.0-alpha would require extra work and testing.

So for now, I'm only going to patch fix [email protected].

To use useEffect in your React Native app safely, just make sure you're using react@^16.7.0-alpha.2 and scheduler@^0.12.0-alpha.3.

The patch release has now been published as [email protected]

[bvaughn]

I also published the fix to [email protected] since it potentially impacts the edge case of react-dom ConcurrentMode and IE9