-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix issue with setImmediate not defined on Web #4276
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tomekzaw
approved these changes
Mar 27, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😎
piaskowyk
approved these changes
Mar 28, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐎
56 tasks
fluiddot
pushed a commit
to wordpress-mobile/react-native-reanimated
that referenced
this pull request
Jun 5, 2023
## Summary This PR removes uses of setImmediate which is not a standard API across web browsers. Instead, we use queueMicrotask API which is available both in the browser world, in React Native and in Node. Fixes software-mansion#4140 ## Test plan Run example app, test WebExample, test repro app from software-mansion#4140 with `import setImmediate` removed
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 22, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary This PR is amending some changes from #4276 where we exchanged `setImmediates` for `queueMicrotask` for `_scheduneOnJS`. `queueMicrotask` does not take any arguments, only a callback and the change of behaviour stemming from this was not considered in `runOnJS` implementation. This PR fixes this and also makes `runOnJS` actually call `_scheduleOnJS` on web, making it asynchronous and consistent with general behavior of `runOnJS` - before it was unreachable code on web since it would simply return the provided function that was called synchronously. ## Test plan Run WebExample and see that now `runOnJS` is working properly. I used the following snippet: ```TypeScript function foo() { console.log('hello'); } function foz(str: string) { console.log(str); } runOnJS(foo)(); runOnJS(foz)('bye'); function bar() { console.log('hello again'); } function baz(str: string) { console.log(str); } runOnUI(() => { runOnJS(bar)(); runOnJS(baz)('bye again'); })(); ``` Before: <img width="738" alt="Screenshot 2023-06-14 at 13 45 59" src="https://github.com/software-mansion/react-native-reanimated/assets/40713406/e57210fb-078f-4c58-badc-9476683695b6"> After: <img width="735" alt="Screenshot 2023-06-14 at 13 45 11" src="https://github.com/software-mansion/react-native-reanimated/assets/40713406/23eb27df-f2db-4e18-ad68-bcae37282e0c">
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR removes uses of setImmediate which is not a standard API across web browsers. Instead, we use queueMicrotask API which is available both in the browser world, in React Native and in Node.
Fixes #4140
Test plan
Run example app, test WebExample, test repro app from #4140 with
import setImmediate
removed