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

Throw error when calling runOnUI on the UI runtime #4477

Merged
merged 3 commits into from
May 24, 2023

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented May 18, 2023

Summary

This PR adds a check that throws an error with a developer-friendly message when trying to call runOnUI function from a worklet which is already running on the UI runtime.

Motivation

It's difficult to decide what's the expected behavior of runOnUI in such case:

  • If the function should be scheduled for the current animation frame, use queueMicrotask.
  • If the function should be scheduled for the next animation frame, use requestAnimationFrame.
  • If the function should be executed immediately, simply call it synchronously.

Note that neither setTimeout nor setInterval are implemented on the UI runtime. Also, setImmediate is not available as well as it's not a part of the standard.

Implementation

Currently, calling runOnUI on the UI thread throws an error "Tried to synchronously call..." because runOnUI is not marked as a worklet. Adding 'worklet'; directive causes "cannot add new property" error because _runOnUIQueue gets frozen while cloning the worklet to the UI runtime. This PR adds _runOnUIQueue to the whitelisted identifiers in Reanimated Babel plugin so it doesn't get captured into the closure, thus doesn't need to be cloned and frozen.

Alternatives

We could replace all occurrences of _runOnUIQueue with global._runOnUIQueue so that the queue isn't captured into worklet closure and thus doesn't get frozen. Another option would be simply to whitelist runOnUI function but it wouldn't work properly when aliased.

Test plan

App.tsx
import { Button, StyleSheet, View } from 'react-native';

import React from 'react';
import { runOnUI } from 'react-native-reanimated';

export default function App() {
  const handlePress = () => {
    runOnUI(() => {
      'worklet';
      console.log(_WORKLET, 'Hello from the UI thread!');
      runOnUI(() => {
        'worklet';
        console.log(_WORKLET, 'Hello from the UI thread again!');
      })();
    })();
  };

  return (
    <View style={styles.container}>
      <Button title="Click me" onPress={handlePress} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});
runOnUI(() => {
  'worklet';
  console.log(_WORKLET, 'Hello from the UI thread!');
  runOnUI(() => {
    'worklet';
    console.log(_WORKLET, 'Hello from the UI thread again!');
  })();
})();

Your beautiful eyes should see the following error:

error

Copy link
Contributor

@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.

👏

@tomekzaw tomekzaw added this pull request to the merge queue May 24, 2023
Merged via the queue into main with commit 2d34dfc May 24, 2023
2 checks passed
@tomekzaw tomekzaw deleted the @tomekzaw/runOnUIonUI branch May 24, 2023 19:16
@tomekzaw tomekzaw mentioned this pull request May 24, 2023
tomekzaw added a commit that referenced this pull request May 24, 2023
## Summary

This PR fixes the following error which appeared after #4477.

<img width="779" alt="error"
src="https://github.com/software-mansion/react-native-reanimated/assets/20516055/4d5f7a2f-1f59-43ac-bce2-7ae8ed6bccf0">

## Test plan

Check if WebExample works.
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
…n#4477)

## Summary

This PR adds a check that throws an error with a developer-friendly
message when trying to call `runOnUI` function from a worklet which is
already running on the UI runtime.

## Motivation

It's difficult to decide what's the expected behavior of `runOnUI` in
such case:
* If the function should be scheduled for the current animation frame,
use `queueMicrotask`.
* If the function should be scheduled for the next animation frame, use
`requestAnimationFrame`.
* If the function should be executed immediately, simply call it
synchronously.

Note that neither `setTimeout` nor `setInterval` are implemented on the
UI runtime. Also, `setImmediate` is not available as well as it's not a
part of the standard.

## Implementation

Currently, calling `runOnUI` on the UI thread throws an error "Tried to
synchronously call..." because `runOnUI` is not marked as a worklet.
Adding `'worklet';` directive causes "cannot add new property" error
because `_runOnUIQueue` gets frozen while cloning the worklet to the UI
runtime. This PR adds `_runOnUIQueue` to the whitelisted identifiers in
Reanimated Babel plugin so it doesn't get captured into the closure,
thus doesn't need to be cloned and frozen.

## Alternatives

We could replace all occurrences of `_runOnUIQueue` with
`global._runOnUIQueue` so that the queue isn't captured into worklet
closure and thus doesn't get frozen. Another option would be simply to
whitelist `runOnUI` function but it wouldn't work properly when aliased.
 
## Test plan

<details>
<summary>App.tsx</summary>

```tsx
import { Button, StyleSheet, View } from 'react-native';

import React from 'react';
import { runOnUI } from 'react-native-reanimated';

export default function App() {
  const handlePress = () => {
    runOnUI(() => {
      'worklet';
      console.log(_WORKLET, 'Hello from the UI thread!');
      runOnUI(() => {
        'worklet';
        console.log(_WORKLET, 'Hello from the UI thread again!');
      })();
    })();
  };

  return (
    <View style={styles.container}>
      <Button title="Click me" onPress={handlePress} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
  },
});
```

</details>

```ts
runOnUI(() => {
  'worklet';
  console.log(_WORKLET, 'Hello from the UI thread!');
  runOnUI(() => {
    'worklet';
    console.log(_WORKLET, 'Hello from the UI thread again!');
  })();
})();
```

Your beautiful eyes should see the following error:

<img width="503" alt="error"
src="https://github.com/software-mansion/react-native-reanimated/assets/20516055/2a158606-7957-432d-9336-ab9ea8323402">
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

This PR fixes the following error which appeared after software-mansion#4477.

<img width="779" alt="error"
src="https://github.com/software-mansion/react-native-reanimated/assets/20516055/4d5f7a2f-1f59-43ac-bce2-7ae8ed6bccf0">

## Test plan

Check if WebExample works.
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.

None yet

3 participants