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

Auto-workletize functions passed to runOnUI #4459

Merged
merged 3 commits into from
May 16, 2023

Conversation

tomekzaw
Copy link
Member

Summary

This PR adds runOnUI to the list of automatically workletized functions as suggested by @kacperkapusciak.

Previously, the following code:

runOnUI(() => {
  console.log('Hello from the UI thread!');
})();

would throw the following error:

Error: runOnUI() can only be used on worklets, js engine: hermes

because the function passed to runOnUI needs to be marked as worklet:

 runOnUI(() => {
+  'worklet';
   console.log('Hello from the UI thread!');
 })();

After this change, the 'worklet'; directive can be omitted and the function will be automatically workletized by the Reanimated Babel plugin.

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(() => {
      console.log('Hello from the UI thread!');
    })();
  };

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

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

@tjzel
Copy link
Contributor

tjzel commented May 16, 2023

While I agree that this change is a natural extension of what we have already done, I wonder if runOnUI has many common use cases. I always thought we should keep the 'worklet' directive here as necessary due to the fact that using this function has its limitations and should be used only when one has a deeper understanding of Reanimated. For example:

runOnUI(() => {
  'worklet';
  console.log('Hello from the UI thread!');
  runOnUI(() => {
    'worklet';
    console.log("Hello again with 'worklet'!");
  })();
})();

and

runOnUI(() => {
  'worklet';
  console.log('Hello from the UI thread!');
  runOnUI(() => {
    // Missing 'worklet' directive
    console.log("Hello again without 'worklet'!");
  })();
})();

Both will crash. If you consider this limitation unrelated to whether we should be forced to type 'worklet' or not, I'm open to approving this.

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.

How about adding one or two unit tests for this?

@tomekzaw tomekzaw requested a review from tjzel May 16, 2023 15:00
@tomekzaw tomekzaw added this pull request to the merge queue May 16, 2023
Merged via the queue into main with commit 1143605 May 16, 2023
7 checks passed
@tomekzaw tomekzaw deleted the @tomekzaw/autoworkletize-runOnUI branch May 16, 2023 15:05
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

This PR adds `runOnUI` to the list of automatically workletized
functions as suggested by @kacperkapusciak.

Previously, the following code:

```tsx
runOnUI(() => {
  console.log('Hello from the UI thread!');
})();
```

would throw the following error:

```
Error: runOnUI() can only be used on worklets, js engine: hermes
```

because the function passed to `runOnUI` needs to be marked as worklet:

```diff
 runOnUI(() => {
+  'worklet';
   console.log('Hello from the UI thread!');
 })();
```

After this change, the `'worklet';` directive can be omitted and the
function will be automatically workletized by the Reanimated Babel
plugin.

## 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(() => {
      console.log('Hello from the UI thread!');
    })();
  };

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

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

</details>
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

4 participants