Add fallback to setImmediate for Expo Router static rendering #4665

javascripter · 2023-07-03T07:58:24Z

Summary

[expo49, router v2] Metro error: requestAnimationFrame is not defined using drawer expo/router#718
- Twitter thread

In Node.js environments such as when static rendering with Expo Router, requestAnimationFrame is not defined. This results in an error when using libraries that depend on react-native-reanimated like Drawer or when using react-native-reanimated directly in Expo Router with static rendering, causing a hydration mismatch due to rendering errors.

In this PR, I added a fallback to setImmediate if requestAnimationFrame is not defined in globalThis. I did not simply replace requestAnimationFrame with setImmediate because setImmediate is slower than requestAnimationFrame in CSR environments. This issue is not applicable to static rendering environments however, so setImmediate is fine there.

Test plan

See this issue's Minimal reproducible example

…ndering with Expo Router)

javascripter · 2023-07-03T09:24:33Z

Not very sure how to fix failing jest tests.

satya164 · 2023-07-03T18:19:45Z

Seems like the actual issue is that scheduleOnUI is being called during render for SSR/static build - which results in calling requestAnimationFrame. Side effects and callbacks shouldn't be called during SSR/static build.

javascripter · 2023-07-04T04:04:06Z

Seems like the actual issue is that scheduleOnUI is being called during render for SSR/static build - which results in calling requestAnimationFrame. Side effects and callbacks shouldn't be called during SSR/static build.

@satya164 I see. If I change the code to the below tests will pass. Do you think the below approach seems more reasonable? If so, I will update both the code and the PR description accordingly.

  scheduleOnUI<T>(worklet: ShareableRef<T>) {
    // In Node.js environments (like when static rendering with Expo Router)
    // requestAnimationFrame is unavailable. Do not call side effects in this case.
    if (typeof requestAnimationFrame === 'undefined') {
      return;
    }
    // @ts-ignore web implementation has still not been updated after the rewrite, this will be addressed once the web implementation updates are ready
    requestAnimationFrame(worklet);
  }

tjzel · 2023-07-05T17:38:21Z

I've tried to understand what's causing the jest problem but still haven't got any solid results.

From what I concluded if we use scheduleOnUI (which comes from scheduleOnUI = requestAnimationFrame) in scheduleOnUI (btw. I think it's a bad idea to have the same name for those) it's causing tests to fail, but if we call directly requestAnimationFrame in scheduleOnUI it's ok.
Probably because scheduleOnUI gets torn down (or torn down before) requestAnimationFrame and is still called? I haven't yet had a great deal of experience of solving Jest issues so I'm not exactly sure what that means.

satya164 · 2023-07-05T18:42:45Z

@javascripter: Do you think the below approach seems more reasonable?

Imo it doesn't address the root of the problem - scheduleOnUI being called during SSR/static build

nDeavor9 · 2023-08-07T04:29:03Z

Sorry to directly comment on this.
Can you guys please let me know when will this PR be merged?
I'm facing the ReferenceError: requestAnimationFrame is not defined at JSReanimated.scheduleOnUI and it's blocking my release.
Hopefully it gets done soon. Thanks.

arivanbastos · 2023-08-07T15:24:01Z

Hi guys. Any workaround to this problem?
Cant release my app due "ReferenceError: requestAnimationFrame is not defined" error.

tomekzaw · 2023-08-09T05:24:20Z

Hey @nDeavor9 and @arivanbastos, can you confirm if this PR resolves the issue?

nDeavor9 · 2023-08-09T05:27:35Z

@tomekzaw I guess so.
As @satya164 said, the root problem might be different, but the error that's blocking from exporting is exactly 'requestAnimationFrame' being undefined.

sabyasachibose-sigfyn · 2023-08-09T06:49:23Z

Our builds are blocked at the moment. Can we please have a resolution on this?

arivanbastos · 2023-08-09T13:50:57Z

@tomekzaw
Edit
I installed the PR by running
npm i software-mansion/react-native-reanimated#pull/4665

And yes, the error is gone.

javascripter · 2023-08-09T16:30:53Z

Sorry, haven't had much time to investigate the root cause further, since I'm not very familiar with reanimated's internals.

I originally posted the patch on Twitter with minimal changes, so if you're blocked on this you can use pnpm patch or patch-package to workaround this issue as well.

Twitter post:
https://twitter.com/javascripter/status/1675549640471760896
Link to patch:
https://gist.github.com/javascripter/4e4e20e9024a33592437648b718c763d

bhyoo99 · 2023-08-11T02:49:56Z

Here is my patch-package file
https://gist.github.com/bhyoo99/f4954316c498f3ca261197166855e21d

✅ expo-router checked
✅ SSG web build checked

tjzel · 2023-08-22T16:14:52Z

Hi @javascripter! I took the liberty to commit to your fork, since I got a solution for this that should both work and pass our tests. Turns out at some point in our code (but after creating scheduleOnUI) we substitute requestAnimationFrame implementation accordingly to how it should be done with React Native when using Jest. Since it's happening later, scheduleOnUI would not have it's implementation substituted.

Please let me know if it works for you - if it does, I think it is safe to merge (after my colleagues review it of course).

tomekzaw · 2023-08-22T17:11:33Z

src/reanimated2/js-reanimated/JSReanimated.ts

+const scheduleOnUI = globalThis.requestAnimationFrame
+  ? isJest()
+    ? mockedRequestAnimationFrame
+    : globalThis.requestAnimationFrame
+  : setImmediate;


First, can we handle Jest first and then the rest?

const scheduleOnUI = isJest() ? mockedRequestAnimationFrame : globalThis.requestAnimationFrame || globalThis.setImmediate;

Can we add a new platform checker isStaticRendering()? This way we could write it this way:

const scheduleOnUI = isJest() ? mockedRequestAnimationFrame : isStaticRendering() ? globalThis.setImmediate : globalThis.requestAnimationFrame;

As far as I understand in this environment useAnimatedFrame is unavailable in Jest also and we want to use setImmediate there. With this implementation we would always use the mock - maybe it's desired - @javascripter could you give more insight on that?

This is hard, we have to discuss it tomorrow.

I used setImmediate in the static rendering environment simply because I tested it in my environment and it seemed to work well for me. As @satya164 said, scheduleOnUI probably doesn't need to execute in that environment so I don't have a concrete reason to prefer setImmediate over the mock in Jest. Always using the mock in Jest seems a reasonable approach to me. @tjzel

Can we check who exactly calls scheduleOnUI here any why? Maybe we can move the call into a conditional. I suspect this might be initializeUIRuntime which calls runOnUIImmediately.

We actually had a quite fruitful discussion about this and we want to go a step further and improve how Reanimated behaves during Static Renders. This PR can stay as a workaround for the moment, but we will open a proper one in the near future.

javascripter · 2023-08-25T17:40:25Z

Hi @javascripter! I took the liberty to commit to your fork, since I got a solution for this that should both work and pass our tests. Turns out at some point in our code (but after creating scheduleOnUI) we substitute requestAnimationFrame implementation accordingly to how it should be done with React Native when using Jest. Since it's happening later, scheduleOnUI would not have it's implementation substituted.

Please let me know if it works for you - if it does, I think it is safe to merge (after my colleagues review it of course).

@tjzel Hi. Thanks for taking a look at this! Yes, this works for me, and the commit looks good to me as well.

kevinhylant · 2023-08-28T16:27:56Z

Looks like there's a conflict — hoping this will get merged soon-ish :)

Thanks for all your work here!

tomekzaw · 2023-08-25T19:17:08Z

src/reanimated2/initializers.ts

-    global.requestAnimationFrame = (callback: (timestamp: number) => void) => {
-      return setTimeout(() => callback(performance.now()), 0);
-    };
+    global.requestAnimationFrame = mockedRequestAnimationFrame;


Since we check globalThis.requestAnimationFrame, shouldn't we also assign to globalThis instead of global?

Suggested change

global.requestAnimationFrame = mockedRequestAnimationFrame;

globalThis.requestAnimationFrame = mockedRequestAnimationFrame;

In our use cases it should be irrelevant what we assign it to according to some docs but I guess it's better practice to use globalThis.

tomekzaw · 2023-08-25T19:20:52Z

src/reanimated2/js-reanimated/JSReanimated.ts

+const scheduleOnUI = globalThis.requestAnimationFrame
+  ? isJest()
+    ? mockedRequestAnimationFrame
+    : globalThis.requestAnimationFrame
+  : setImmediate;


Can we check who exactly calls scheduleOnUI here any why? Maybe we can move the call into a conditional. I suspect this might be initializeUIRuntime which calls runOnUIImmediately.

tomekzaw · 2023-09-05T09:08:17Z

src/reanimated2/js-reanimated/JSReanimated.ts

@@ -28,7 +38,7 @@ export default class JSReanimated {

  scheduleOnUI<T>(worklet: ShareableRef<T>) {
    // @ts-ignore web implementation has still not been updated after the rewrite, this will be addressed once the web implementation updates are ready
-    requestAnimationFrame(worklet);
+    _scheduleOnUI(worklet);


In our codebase the leading underscore means that the function is a JSI binding. Can we rename it to something else?

src/reanimated2/js-reanimated/JSReanimated.ts

https://blog.expo.dev/expo-sdk-49-c6d398cdf740 Having issue in web - expo/expo#23412 - software-mansion/react-native-reanimated#4665 ## Issue Upgrading expo metro as suggested in expo/router#864 (comment) causes problem when running app in `iOS` (not yet tested in android). It freezes in `SplashScreen`. So we need to becareful in merging this `PR`. Currently, it is only working in `web` and freezes in `iOS`.

Fallback to setImmediate in Node.js environments (e.g. when static re…

1702a27

…ndering with Expo Router)

javascripter force-pushed the fix-web-nodejs-env branch from 78e40f3 to 1702a27 Compare July 3, 2023 08:14

Fix for test

6963897

bhyoo99 mentioned this pull request Jul 3, 2023

[expo49, router v2] Metro error: requestAnimationFrame is not defined using drawer expo/router#718

Closed

nDeavor9 mentioned this pull request Aug 7, 2023

ReferenceError: requestAnimationFrame is not defined #4890

Closed

marklawlor mentioned this pull request Aug 17, 2023

Expo web does not run on SDK 49 expo/expo#23412

Closed

hyochan mentioned this pull request Aug 21, 2023

Migrate expo to SDK 49 hyochan/expo-router-starter#5

Merged

working prototype

d43434d

tomekzaw reviewed Aug 22, 2023

View reviewed changes

tjzel added 2 commits September 4, 2023 19:06

mergele main

a19a756

ready to mergele

407d0fe

tomekzaw approved these changes Sep 4, 2023

View reviewed changes

review changes

0f2d272

tomekzaw reviewed Sep 5, 2023

View reviewed changes

rename

b453f16

tomekzaw reviewed Sep 5, 2023

View reviewed changes

src/reanimated2/js-reanimated/JSReanimated.ts Outdated Show resolved Hide resolved

src/reanimated2/js-reanimated/JSReanimated.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

ea40adc

tomekzaw approved these changes Sep 5, 2023

View reviewed changes

tjzel added this pull request to the merge queue Sep 5, 2023

Merged via the queue into software-mansion:main with commit bf59966 Sep 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fallback to setImmediate for Expo Router static rendering #4665

Add fallback to setImmediate for Expo Router static rendering #4665

javascripter commented Jul 3, 2023

javascripter commented Jul 3, 2023

satya164 commented Jul 3, 2023

javascripter commented Jul 4, 2023 •

edited

Loading

tjzel commented Jul 5, 2023

satya164 commented Jul 5, 2023

nDeavor9 commented Aug 7, 2023

arivanbastos commented Aug 7, 2023

tomekzaw commented Aug 9, 2023

nDeavor9 commented Aug 9, 2023

sabyasachibose-sigfyn commented Aug 9, 2023

arivanbastos commented Aug 9, 2023 •

edited

Loading

javascripter commented Aug 9, 2023 •

edited

Loading

bhyoo99 commented Aug 11, 2023

tjzel commented Aug 22, 2023 •

edited

Loading

tomekzaw Aug 22, 2023 •

edited

Loading

tjzel Aug 22, 2023

javascripter Aug 25, 2023

tomekzaw Aug 25, 2023

tjzel Aug 28, 2023

javascripter commented Aug 25, 2023

kevinhylant commented Aug 28, 2023

tomekzaw Aug 25, 2023

tjzel Sep 5, 2023

tomekzaw Aug 25, 2023

tomekzaw Sep 5, 2023

	global.requestAnimationFrame = mockedRequestAnimationFrame;
	globalThis.requestAnimationFrame = mockedRequestAnimationFrame;

Add fallback to setImmediate for Expo Router static rendering #4665

Add fallback to setImmediate for Expo Router static rendering #4665

Conversation

javascripter commented Jul 3, 2023

Summary

Test plan

javascripter commented Jul 3, 2023

satya164 commented Jul 3, 2023

javascripter commented Jul 4, 2023 • edited Loading

tjzel commented Jul 5, 2023

satya164 commented Jul 5, 2023

nDeavor9 commented Aug 7, 2023

arivanbastos commented Aug 7, 2023

tomekzaw commented Aug 9, 2023

nDeavor9 commented Aug 9, 2023

sabyasachibose-sigfyn commented Aug 9, 2023

arivanbastos commented Aug 9, 2023 • edited Loading

javascripter commented Aug 9, 2023 • edited Loading

bhyoo99 commented Aug 11, 2023

tjzel commented Aug 22, 2023 • edited Loading

tomekzaw Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javascripter commented Aug 25, 2023

kevinhylant commented Aug 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javascripter commented Jul 4, 2023 •

edited

Loading

arivanbastos commented Aug 9, 2023 •

edited

Loading

javascripter commented Aug 9, 2023 •

edited

Loading

tjzel commented Aug 22, 2023 •

edited

Loading

tomekzaw Aug 22, 2023 •

edited

Loading