Skip to content

Commit 6bff08e

Browse files
committed
fix: clean up runtime refactor paths
1 parent fd62fdb commit 6bff08e

5 files changed

Lines changed: 43 additions & 29 deletions

File tree

NativeScript/napi/hermes/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Hermes Node-API bridge
2+
3+
`jsr.cpp` is shared by Apple Hermes builds and Android Hermes builds, but the
4+
prebuilt Hermes libraries currently expose different header surfaces.
5+
6+
- `include` is the Apple/current Hermes fallback header set used when
7+
`Frameworks/hermes-headers` is not available.
8+
- `include_old` matches Android's legacy `HERMES` prebuilts and their
9+
`HermesRuntime::createNapiEnv` API.
10+
- `include_shermes` matches Android's `SHERMES` prebuilts and their
11+
`hermes_node_api.h` API.
12+
13+
Keep these directories aligned with the matching prebuilts. Do not mix headers
14+
from one Hermes variant with libraries from another variant.

NativeScript/runtime/android/modules/module/ModuleInternal.cpp

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -377,12 +377,7 @@ napi_value ModuleInternal::LoadModule(napi_env env, const std::string& modulePat
377377
if (pendingException) {
378378
return nullptr;
379379
}
380-
napi_value error = nullptr;
381-
if (error) {
382-
throw NativeScriptException(env, error, "Error running script " + modulePath);
383-
} else {
384-
throw NativeScriptException("Error running script " + modulePath);
385-
}
380+
throw NativeScriptException("Error running script " + modulePath);
386381
}
387382
} else if (Util::EndsWith(modulePath, ".so")) {
388383
auto handle = dlopen(modulePath.c_str(), RTLD_NOW);
@@ -438,18 +433,19 @@ napi_value ModuleInternal::LoadModule(napi_env env, const std::string& modulePat
438433
napi_status status = napi_call_function(env, thiz, moduleFunc, 5, requireArgs, &callResult);
439434
bool pendingException;
440435
napi_is_exception_pending(env, &pendingException);
441-
if (status != napi_ok || pendingException) {
442-
if (pendingException) {
443-
return nullptr;
444-
}
445-
napi_value exception;
446-
napi_get_and_clear_last_exception(env, &exception);
447-
if (exception) {
448-
throw NativeScriptException(env, exception, "Error calling module function: ");
449-
} else {
450-
throw NativeScriptException("Error calling module function: " + modulePath);
451-
}
452-
}
436+
if (status != napi_ok || pendingException) {
437+
if (pendingException) {
438+
return nullptr;
439+
}
440+
441+
napi_value exception = nullptr;
442+
napi_get_and_clear_last_exception(env, &exception);
443+
if (exception) {
444+
throw NativeScriptException(env, exception, "Error calling module function: ");
445+
} else {
446+
throw NativeScriptException("Error calling module function: " + modulePath);
447+
}
448+
}
453449

454450
tempModule.SaveToCache();
455451
result = moduleObj;

metadata-generator/build-step-metadata-generator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ def is_nativescript_source_root(search_path):
172172
strict_includes = env_or_none("NS_DEBUG_METADATA_STRICT_INCLUDES") or env_or_none("TNS_DEBUG_METADATA_STRICT_INCLUDES")
173173
signature_bindings_cpp_path = env_or_none("NS_SIGNATURE_BINDINGS_CPP_PATH") or env_or_none("TNS_SIGNATURE_BINDINGS_CPP_PATH")
174174
if signature_bindings_cpp_path is None:
175-
default_signature_bindings_path = os.path.join(src_root, "NativeScript", "ffi", "napi", "GeneratedSignatureDispatch.inc")
175+
default_signature_bindings_path = os.path.join(src_root, "NativeScript", "ffi", "objc", "napi", "GeneratedSignatureDispatch.inc")
176176
if os.path.isdir(os.path.dirname(default_signature_bindings_path)):
177177
signature_bindings_cpp_path = default_signature_bindings_path
178178

platforms/android/docs/extending-inspector.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ This step involves fetching and building the V8 projectory, explanation for whic
4949

5050
4. Run the ninja build. Upon completion, the inspector protocol files would be at `outgn/$ARCH-release/gen/src/inspector/protocol`.
5151

52-
5. Copy-Paste all `.cpp` and `.h` files in the runtime project at `test-app/runtime/src/main/cpp/v8_inspector/src/inspector/protocol`
52+
5. Copy-paste all `.cpp` and `.h` files into `NativeScript/napi/android/v8/v8_inspector/src/inspector/protocol`.
5353

54-
6. Create a new C++ class extending the desired Domain (e.g. DOM). Name it according to the convention already established by the V8 team - `v8-<domain>-agent-impl.h/cpp` - See [v8-dom-agent-impl.h](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-dom-agent-impl.h#L18). Implement the Backend::<Domain>'s methods in the `.cpp` file.
54+
6. Create a new C++ class extending the desired Domain (e.g. DOM). Name it according to the convention already established by the V8 team - `v8-<domain>-agent-impl.h/cpp` - under `NativeScript/napi/android/v8/v8_inspector/src/inspector`. Implement the Backend::<Domain>'s methods in the `.cpp` file.
5555

56-
7. To implement event handlers of a certain domain, check out <Protocol>DomainCallbackHandlers ([DOMDomainCallbackHandlers](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/DOMDomainCallbackHandlers.h#L14)) which are registered in [JsV8InspectorClient.cpp](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/JsV8InspectorClient.cpp#L237)
56+
7. To implement event handlers for a domain, add the runtime-side callback handlers under `NativeScript/runtime/android/inspector` and register them in `NativeScript/runtime/android/inspector/JsV8InspectorClient.cpp`.
5757

58-
8. Register the newly created agent implementations in [v8-inspector-session-impl.h/cc](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.h#L19) - `V8InspectorSessionImpl` class:
58+
8. Register the newly created agent implementations in `NativeScript/napi/android/v8/v8_inspector/src/inspector/v8-inspector-session-impl.h/cc` - `V8InspectorSessionImpl` class:
5959

6060
8.1. Changes in v8-inspector-session-impl.`h`
6161
- `#include "src/inspector/protocol/<Domain>.h`
@@ -65,13 +65,13 @@ This step involves fetching and building the V8 projectory, explanation for whic
6565

6666
8.2. Changes in v8-inspector-session-impl.`cc`
6767
- `#include "src/inspector/protocol/v8-<domain>-agent-impl.h`
68-
- register protocol domain command prefix in [canDispatchMethod()](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L31)
69-
- in the V8InspectorSessionImpl constructor initialize the new agent implementation with a [nullptr](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L86)
70-
- [create a unique pointer wrapper for your new agent impl instance](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L141), and call the static domain dispatcher's [wire](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L143) method
71-
- make sure to call [agent.disable](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L167) in V8InspectorSessionImpl's destructor.
72-
- register the newly implemented domain as a supported one inside [ V8InspectorSessionImpl::supportedDomainsImpl()](https://github.com/NativeScript/android-runtime/blob/5a04e09439e2bc6a201577895b9ac6538441e758/test-app/runtime/src/main/cpp/v8_inspector/src/inspector/v8-inspector-session-impl.cc#L389)
68+
- register protocol domain command prefix in `canDispatchMethod()`
69+
- in the `V8InspectorSessionImpl` constructor initialize the new agent implementation with `nullptr`
70+
- create a unique pointer wrapper for your new agent impl instance, and call the static domain dispatcher's `wire` method
71+
- make sure to call `agent.disable` in `V8InspectorSessionImpl`'s destructor
72+
- register the newly implemented domain as a supported one inside `V8InspectorSessionImpl::supportedDomainsImpl()`
7373
- `#include "NSV8DebuggerAgentImpl.h"`
7474
- replace **V8DebuggerAgentImpl** with **NSV8DebuggerAgentImpl**
7575

76-
9. Don't forget to add the new classes to the CMakeLists!
76+
9. Don't forget to add the new classes to `platforms/android/test-app/runtime/CMakeLists.txt`.
7777
11. Test whether the new domain agent is registered by opening Chrome DevTools. In order to debug the Chrome DevTools frontend, you could open the Chrome DevTools inside an Android Chrome DevTools session - Ctrl(Cmd) + Shift + I.

platforms/android/test-app/runtime/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ if (PRIMJS)
166166
endif ()
167167

168168
if (HERMES)
169+
# Android legacy Hermes prebuilts expose HermesRuntime::createNapiEnv in
170+
# the older header surface. Keep this separate from Apple/current Hermes.
169171
include_directories(
170172
${NS_NAPI_DIR}/hermes
171173
${NS_NAPI_DIR}/hermes/include_old
@@ -177,6 +179,8 @@ if (HERMES)
177179
endif ()
178180

179181
if (SHERMES)
182+
# Android SHERMES prebuilts expose hermes_node_api.h, so they need the
183+
# matching header surface even though jsr.cpp is shared.
180184
include_directories(
181185
${NS_NAPI_DIR}/hermes
182186
${NS_NAPI_DIR}/hermes/include_shermes

0 commit comments

Comments
 (0)