feat: reloadApplication for JS bundle restart without restarting app process - #1963
Open
NathanWalker wants to merge 3 commits into
Open
feat: reloadApplication for JS bundle restart without restarting app process#1963NathanWalker wants to merge 3 commits into
NathanWalker wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds public Java reload APIs, main-thread restart scheduling, Java and native runtime teardown, and main-runtime lifecycle reset handling. It also updates timezone receiver registration to use cached application context state. ChangesApplication reload and runtime teardown
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds programmatic JavaScript bundle restarts without restarting the app process, and no actionable merge-blocking risk remains; it is merge-ready after normal checks. Sequence Diagram(s)sequenceDiagram
participant NativeScriptRuntime
participant RuntimeHelper
participant Runtime
participant NativeTerminationCallback
participant V8Runtime
NativeScriptRuntime->>RuntimeHelper: reloadApplication()
RuntimeHelper->>Runtime: destroyMainRuntime()
Runtime->>NativeTerminationCallback: TerminateRuntimeCallback(runtimeId)
NativeTerminationCallback->>V8Runtime: terminate workers and destroy isolate
NativeTerminationCallback-->>Runtime: native termination completes
RuntimeHelper->>RuntimeHelper: initialize and run replacement runtime
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NathanWalker
marked this pull request as ready for review
June 25, 2026 19:05
Contributor
Author
|
@copilot resolve the merge conflicts in this pull request |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Resolved and pushed via merge commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test-app/app/src/main/java/com/tns/NativeScriptRuntime.java`:
- Around line 11-12: The NativeScriptRuntime.reloadApplication(String baseDir)
overload currently drops its baseDir argument, so callers cannot reload from a
chosen bundle location. Update this method to either pass baseDir through to a
new RuntimeHelper.reloadApplication(String baseDir) implementation that rebuilds
the runtime config from that directory, or remove the overload if it is not
meant to be supported; keep the API behavior aligned with the method signature
and related RuntimeHelper reload logic.
In `@test-app/app/src/main/java/com/tns/RuntimeHelper.java`:
- Around line 340-349: `registerTimezoneChangedListener` is unregistering
`timezoneChangedReceiver` with whatever `context` is passed in, but that can
differ between the initial call and `reloadApplication`, causing the old
receiver to remain registered. Update the listener bookkeeping so the same
Context instance used for registration is also used for `unregisterReceiver`
(for example by storing the registering Context alongside
`timezoneChangedReceiver` or consistently using `applicationContext` in
`registerTimezoneChangedListener` and its callers). Keep the existing
`IllegalArgumentException` handling only for truly stale registrations, and
ensure the old receiver is fully removed before assigning and registering the
new one.
In `@test-app/runtime/src/main/cpp/com_tns_Runtime.cpp`:
- Around line 358-362: The null-runtime checks in the affected callbacks do not
stop execution, so `runtime->GetIsolate()` can still be called on a null pointer
and crash. In each callback that resolves a `runtime` and then uses it
(`com_tns_Runtime` helpers around the reported branches), add an immediate
return inside the `if (runtime == nullptr)` path before any dereference, and
apply the same early-exit pattern consistently to all listed null-runtime
branches.
In `@test-app/runtime/src/main/java/com/tns/Runtime.java`:
- Around line 478-500: The termination flow in Runtime.destroyMainRuntime() and
terminateWorkers() is racing because the TerminateThread message is posted
asynchronously and workerIdToHandler is cleared immediately afterward. Add a
blocking handshake so terminateWorkers() waits until each worker has actually
processed the termination signal before destroyMainRuntime() continues with
runtimeCache.remove(), pendingWorkerMessages.clear(), currentRuntime.remove(),
and TerminateRuntimeCallback. Use the existing Runtime and workerHandler
messaging path to coordinate completion, and only clear workerIdToHandler after
all workers confirm shutdown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e52793de-9a24-4365-9c36-5c412d5895b3
📒 Files selected for processing (4)
test-app/app/src/main/java/com/tns/NativeScriptRuntime.javatest-app/app/src/main/java/com/tns/RuntimeHelper.javatest-app/runtime/src/main/cpp/com_tns_Runtime.cpptest-app/runtime/src/main/java/com/tns/Runtime.java
…process Helpful for programmatic reset of JS isolate for clean restart of JS application as well as OTA (over-the-air) updates without restarting the entire app process.
NathanWalker
force-pushed
the
feat/reload-app
branch
from
August 15, 2026 02:09
0bc3036 to
2bb05a8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This file contains hidden or 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
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.
Helpful for programmatic reset of JS isolate for clean restart of JS application as well as OTA (over-the-air) updates without restarting the entire app process.
NativeScript/ios#384
Summary by CodeRabbit