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

Fix V8 integration for Reanimated V3 #3736

Merged

Conversation

capezzbr
Copy link
Contributor

@capezzbr capezzbr commented Nov 2, 2022

Description

This PR is an attempt to fix V8 integration with Reanimated V3. Current issue:

react-native/node_modules/react-native-reanimated/Common/cpp/ReanimatedRuntime/ReanimatedRuntime.cpp:40:38: error: use of undeclared identifier 'runtime_'

Changes

Updated ReanimatedRuntime::make to accept a new parameter jsi::Runtime *runtime, which is necessary to initialize V8

Test code and steps to reproduce

  • Create a new RN app
  • Setup the app to have V8 as engine
  • Enable the new architecture (might not be necessary)
  • Build the Android app
  • Confirm the app builds and runs fine with this change

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

@capezzbr capezzbr marked this pull request as ready for review November 2, 2022 23:05
@piaskowyk piaskowyk self-requested a review November 3, 2022 08:52
@piaskowyk piaskowyk self-assigned this Nov 3, 2022
- 'Example/package.json'
push:
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to change it to Reanimated2 when cherry-picking this PR.

.github/workflows/build-v8.yml Outdated Show resolved Hide resolved
.github/workflows/build-v8.yml Outdated Show resolved Hide resolved
.github/workflows/helper/configureV8.js Outdated Show resolved Hide resolved
Common/cpp/ReanimatedRuntime/ReanimatedRuntime.cpp Outdated Show resolved Hide resolved
.github/workflows/build-v8.yml Outdated Show resolved Hide resolved
@piaskowyk piaskowyk requested a review from tomekzaw November 4, 2022 13:41
.github/workflows/helper/configureV8.js Outdated Show resolved Hide resolved
.github/workflows/helper/configureV8.js Outdated Show resolved Hide resolved
@piaskowyk piaskowyk merged commit db71fa1 into software-mansion:main Nov 7, 2022
piaskowyk pushed a commit that referenced this pull request Nov 7, 2022
This PR is an attempt to fix V8 integration with Reanimated V3. Current issue:

```
react-native/node_modules/react-native-reanimated/Common/cpp/ReanimatedRuntime/ReanimatedRuntime.cpp:40:38: error: use of undeclared identifier 'runtime_'
```

Updated `ReanimatedRuntime::make` to accept a new parameter `jsi::Runtime *runtime`, which is necessary to initialize V8

- Create a new RN app
- Setup the app to have V8 as engine
- Enable the new architecture (might not be necessary)
- Build the Android app
- Confirm the app builds and runs fine with this change

<!--
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)
-->

- [ ] 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
@capezzbr capezzbr deleted the capezzb/fix-v8-with-reanimatedv3 branch November 7, 2022 19:18
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Description

This PR is an attempt to fix V8 integration with Reanimated V3. Current issue:

```
react-native/node_modules/react-native-reanimated/Common/cpp/ReanimatedRuntime/ReanimatedRuntime.cpp:40:38: error: use of undeclared identifier 'runtime_'
```

## Changes

Updated `ReanimatedRuntime::make` to accept a new parameter `jsi::Runtime *runtime`, which is necessary to initialize V8

## Test code and steps to reproduce

- Create a new RN app
- Setup the app to have V8 as engine
- Enable the new architecture (might not be necessary)
- Build the Android app
- Confirm the app builds and runs fine with this change

<!--
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants