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

[Web] setImmediate SSR regression in 2.3.0-beta.3 #2621

Closed
1 of 3 tasks
nandorojo opened this issue Nov 10, 2021 · 3 comments · Fixed by #2622
Closed
1 of 3 tasks

[Web] setImmediate SSR regression in 2.3.0-beta.3 #2621

nandorojo opened this issue Nov 10, 2021 · 3 comments · Fixed by #2622

Comments

@nandorojo
Copy link
Contributor

nandorojo commented Nov 10, 2021

Description

If you run 2.3.0-beta.3 with Next.js, you get this error:

image

Expected behavior

setImmediate should not be used on Web. Instead, it should use requestAnimationFrame. This is a regression from previous versions, where this replacement was already made.

It was originally fixed here: #1891

Actual behavior & steps to reproduce

Create an SSR app, use reanimated 2.3.0-beta.3.

Snack or minimal code example

I can create one if necessary, but this bug has historically been an issue with Reanimated.

Package versions

  • React Native: 0.64.3
  • React Native Reanimated: 2.3.0-beta.3
  • NodeJS:
  • Xcode:
  • Java & Gradle:

Affected platforms

  • Android
  • iOS
  • Web

Patch

This patch fixes it:

diff --git a/node_modules/react-native-reanimated/lib/reanimated2/ViewDescriptorsSet.js b/node_modules/react-native-reanimated/lib/reanimated2/ViewDescriptorsSet.js
index 2db94aa..3f1d699 100644
--- a/node_modules/react-native-reanimated/lib/reanimated2/ViewDescriptorsSet.js
+++ b/node_modules/react-native-reanimated/lib/reanimated2/ViewDescriptorsSet.js
@@ -1,5 +1,14 @@
 import { useRef } from 'react';
 import { makeMutable } from './core';
+import { Platform } from 'react-native';
+
+let updater
+if (Platform.OS === 'web') {
+  updater = requestAnimationFrame
+} else {
+  updater = setImmediate
+}
+
 export function makeViewDescriptorsSet() {
     const ref = useRef(null);
     if (ref.current === null) {
@@ -18,7 +27,7 @@ export function makeViewDescriptorsSet() {
                 data.items.push(item);
                 if (!data.waitForInsertSync) {
                     data.waitForInsertSync = true;
-                    setImmediate(() => {
+                    updater(() => {
                         data.sharableViewDescriptors.value = data.items;
                         data.waitForInsertSync = false;
                     });
@@ -28,7 +37,7 @@ export function makeViewDescriptorsSet() {
                 data.batchToRemove.add(viewTag);
                 if (!data.waitForRemoveSync) {
                     data.waitForRemoveSync = true;
-                    setImmediate(() => {
+                    updater(() => {
                         const items = [];
                         for (const item of data.items) {
                             if (data.batchToRemove.has(item.tag)) {
@github-actions
Copy link

Issue validator

The issue is valid!

nandorojo added a commit to nandorojo/react-native-reanimated that referenced this issue Nov 10, 2021
Use `requestAnimationFrame` on Web, instead of `setImmediate`.

See software-mansion#2621 and software-mansion#1521 for reference.
@nandorojo
Copy link
Contributor Author

Opened a PR to solve this at #2622

@piaskowyk
Copy link
Member

Thanks to report and PR 😍 I will test it before release and I will merge it if everything will be ok.

@piaskowyk piaskowyk self-assigned this Nov 12, 2021
piaskowyk pushed a commit that referenced this issue Nov 23, 2021
Use `requestAnimationFrame` on Web, instead of `setImmediate`.

See #2621 and #1521 for reference.

## Description

Fix #2621
<!--
Description and motivation for this PR.

Inlude Fixes #<number> if this is fixing some issue.

Fixes # .
-->

## Changes

`setImmediate` breaks SSR on Web. `requestAnimationFrame` is better. This was already discussed and merged previously at #1521.

<!--
Please describe things you've changed here, make a **high level** overview, if change is simple you can omit this section.

For example:

- Added `foo` method which add bouncing animation
- Updated `about.md` docs
- Added caching in CI builds

-->

<!--

## Screenshots / GIFs

N/A.

### Before

Next.js wouldn't compile.

### After

It now does compile.

-->

## Test code and steps to reproduce

<!--
Please include code that can be used to test this change and short description how this example should work.
This snippet should be as minimal as possible and ready to be pasted into editor (don't exclude exports or remove "not important" parts of reproduction example)
-->

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants