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 runtime creation #2047

Merged
merged 2 commits into from
May 26, 2021
Merged

Fix runtime creation #2047

merged 2 commits into from
May 26, 2021

Conversation

piaskowyk
Copy link
Member

@piaskowyk piaskowyk commented May 26, 2021

Description

Replacement for PR #1960

Checklist

  • Ensured that CI passes

@piaskowyk piaskowyk merged commit b9532cf into master May 26, 2021
@piaskowyk piaskowyk deleted the @piaskowyk/runtimes branch May 26, 2021 09:19
piaskowyk pushed a commit that referenced this pull request Sep 22, 2022
## Description

This PR is related to #1960 (and its replacement #2047). It adds the
ability to use Chrome DevTools to add breakpoints and debug worklets (or
the UI context in general) both on Android and iOS.

## Major changes

Runtime creation has been moved to `ReanimatedRuntime` for both Android
and iOS (in the `Common/cpp/ReanimatedRuntime` directory).

Before (Android) [file: `NativeProxy.cpp`]:
```cpp
#if JS_RUNTIME_HERMES
  auto config =
      ::hermes::vm::RuntimeConfig::Builder().withEnableSampleProfiling(false);
  std::shared_ptr<jsi::Runtime> animatedRuntime =
      facebook::hermes::makeHermesRuntime(config.build());
#elif JS_RUNTIME_V8
      …
#else
      …
#endif
```

After (shared) [file: `NativeProxy.cpp`]:
```cpp
ReanimatedRuntime::make(jsQueue);
```

The custom build config has been removed, as sample profiling is set to
false by default.

The created `HermesRuntimeManager` object is the stored inside
`ReanimatedNativeModule` as it is important that it’s lifetime is synced
with the lifetime of the module.

## Testing

- [x] Builds on Android
- [x] Builds on Android in release mode
- [x] App reloads work on Android
- [x] Builds on iOS
- [x] Builds on iOS in release mode
- [x] App reloads work on iOS
- [x] JSC still works
- [x] JSC debugging in Safari with iOS still works
- [x] Paper still works

Versions of React-Native tested: 0.70

## Things to look into

- ~~Breakpoint labels do not appear on iOS~~
_I tested this on a new app with Flipper on the main JS thread, and it
was also the case, so it seem to not be an issue on our side_
- ~~After removing a breakpoint on iOS it can't be set in the same
location again~~
_This seems to be an issue with Chrome DevTools as connecting to
Reanimated's runtime through Flipper fully works_
- ~~Edge-case: the app will crash if a reload is performed while the
debugger is open~~
_Will be fixed in a PR to metro and
[58e9e7b](https://github.com/software-mansion/react-native-reanimated/pull/3526/commits/58e9e7bcd7d3b6b590a5f6fca0db93a951eaa39e)_
- The latest release of Chrome (105) broke source maps, so only Flipper
works now. I reported the issue
[here](https://bugs.chromium.org/p/chromium/issues/detail?id=1360298#makechanges),
and I'm waiting for a response from the Chromium team

## TODO

- [x] Open PR to update debugging docs in documentation (will be
included in #3446)
- [x] Open PR in [facebook/flipper](https://github.com/facebook/flipper)
to enable Flipper support on custom runtimes (facebook/flipper#4047)
- [x] Open PR in [facebook/metro](https://github.com/facebook/metro) to
support debugger reloads on custom runtimes (facebook/metro#864)
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
…are-mansion#3526)

## Description

This PR is related to software-mansion#1960 (and its replacement software-mansion#2047). It adds the
ability to use Chrome DevTools to add breakpoints and debug worklets (or
the UI context in general) both on Android and iOS.

## Major changes

Runtime creation has been moved to `ReanimatedRuntime` for both Android
and iOS (in the `Common/cpp/ReanimatedRuntime` directory).

Before (Android) [file: `NativeProxy.cpp`]:
```cpp
#if JS_RUNTIME_HERMES
  auto config =
      ::hermes::vm::RuntimeConfig::Builder().withEnableSampleProfiling(false);
  std::shared_ptr<jsi::Runtime> animatedRuntime =
      facebook::hermes::makeHermesRuntime(config.build());
#elif JS_RUNTIME_V8
      …
#else
      …
#endif
```

After (shared) [file: `NativeProxy.cpp`]:
```cpp
ReanimatedRuntime::make(jsQueue);
```

The custom build config has been removed, as sample profiling is set to
false by default.

The created `HermesRuntimeManager` object is the stored inside
`ReanimatedNativeModule` as it is important that it’s lifetime is synced
with the lifetime of the module.

## Testing

- [x] Builds on Android
- [x] Builds on Android in release mode
- [x] App reloads work on Android
- [x] Builds on iOS
- [x] Builds on iOS in release mode
- [x] App reloads work on iOS
- [x] JSC still works
- [x] JSC debugging in Safari with iOS still works
- [x] Paper still works

Versions of React-Native tested: 0.70

## Things to look into

- ~~Breakpoint labels do not appear on iOS~~
_I tested this on a new app with Flipper on the main JS thread, and it
was also the case, so it seem to not be an issue on our side_
- ~~After removing a breakpoint on iOS it can't be set in the same
location again~~
_This seems to be an issue with Chrome DevTools as connecting to
Reanimated's runtime through Flipper fully works_
- ~~Edge-case: the app will crash if a reload is performed while the
debugger is open~~
_Will be fixed in a PR to metro and
[58e9e7b](https://github.com/software-mansion/react-native-reanimated/pull/3526/commits/58e9e7bcd7d3b6b590a5f6fca0db93a951eaa39e)_
- The latest release of Chrome (105) broke source maps, so only Flipper
works now. I reported the issue
[here](https://bugs.chromium.org/p/chromium/issues/detail?id=1360298#makechanges),
and I'm waiting for a response from the Chromium team

## TODO

- [x] Open PR to update debugging docs in documentation (will be
included in software-mansion#3446)
- [x] Open PR in [facebook/flipper](https://github.com/facebook/flipper)
to enable Flipper support on custom runtimes (facebook/flipper#4047)
- [x] Open PR in [facebook/metro](https://github.com/facebook/metro) to
support debugger reloads on custom runtimes (facebook/metro#864)
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.

2 participants