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

Pass down generator boolean in babel plugin #5565

Merged
merged 6 commits into from
Jan 11, 2024

Conversation

wcandillon
Copy link
Contributor

fixes #5564

@wcandillon wcandillon closed this Jan 9, 2024
@wcandillon wcandillon reopened this Jan 9, 2024
@wcandillon
Copy link
Contributor Author

As far as I tested, this PR is enough to have full generator support with worklets which I'm quite excited about.

@tomekzaw tomekzaw requested a review from tjzel January 9, 2024 14:35
Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Could you also please add a short unit/shapshot test?

cc @tjzel

@wcandillon
Copy link
Contributor Author

if you give me some pointer on how to do it, I'd be happy to.

@tomekzaw
Copy link
Member

tomekzaw commented Jan 9, 2024

You can just paste some input source code that doesn't work without this change and we'll take care of it!

Also, here is a sample unit test. You can run it with yarn test in the root directory.

edit: oh, I missed the example from the issue description, sorry about that. Let me know if you wish to add this unit test, if not we can do that for you 😄

@wcandillon
Copy link
Contributor Author

@tomekzaw I added a little test ☺️

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks! Awaiting final approval from @tjzel

@tjzel
Copy link
Collaborator

tjzel commented Jan 11, 2024

Everything looks good, I just added one more test 😺

@tjzel tjzel added this pull request to the merge queue Jan 11, 2024
Merged via the queue into software-mansion:main with commit 2b6d51b Jan 11, 2024
7 checks passed
@colinta
Copy link

colinta commented Jan 11, 2024

I'm really excited for this – @wcandillon and I were exploring using generators as a more fluent way to compose functions that need to call out to a runtime, and realized that it was especially well suited to animation code! 🥳

github-merge-queue bot pushed a commit that referenced this pull request Jan 12, 2024
## Summary

Inspired by #5565. Currently we don't respect if the workletized
function is an async one. This PR fixes this.

Keep in mind that even with these change **async worklets will not work
properly on the UI thread**. This is because Hermes doesn't support them
out of the box and they need to be transformed into generators first
(you can see that in the snapshots).

Properly transforming them so they are valid on UI thread should be
considered a future, very low priority task, since it wasn't requested
yet by our users.

## Test plan

Relevant tests were added to `plugin.test.ts`.

<details>
<summary>
To see the difference between JS and UI behavior
</summary>

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

import React from 'react';

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

export default function EmptyExample() {
  async function asyncWorklet() {
    'worklet';
    await Promise.resolve();
    console.log('Hello world!');
  }

  const runAsyncWorklet = runOnUI(asyncWorklet);

  return (
    <View style={styles.container}>
      <Button title="Run async worklet on UI" onPress={runAsyncWorklet} />
      <Button title="Run async worklet on JS" onPress={asyncWorklet} />
    </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.

Generator asterisk not preserved in the babel plugin
5 participants