feat(harmony): implement in-memory soft reload using devToolsController RELOAD - #588
Conversation
📝 WalkthroughWalkthroughTwo independent refactors in the Harmony pushy module: ChangesBundle Path Caching Removal
Reload Bridge Introduction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
harmony/pushy/src/main/ets/PushyTurboModule.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: .eslintrc.js » 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@harmony/pushy/src/main/ets/PushyTurboModule.ts`:
- Around line 68-75: The reloadBridge path currently assumes
devToolsController.eventEmitter.emit("RELOAD", ...) will always succeed, so
reloadUpdate() and restartApp() can fail instead of using the restartAbility
fallback. Update reloadBridge() to guard the RELOAD branch in PushyTurboModule
by checking that eventEmitter exists and wrapping the emit call in error
handling; if emit is unavailable or throws, fall back to restartAbility() just
like the else path. Keep the logic localized to reloadBridge() so callers like
reloadUpdate() and restartApp() continue to resolve through the fallback.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1338b90d-4945-4f9a-9186-f19ca1ef074a
📒 Files selected for processing (2)
harmony/pushy/src/main/ets/PushyFileJSBundleProvider.etsharmony/pushy/src/main/ets/PushyTurboModule.ts
| private async reloadBridge(): Promise<void> { | ||
| const devToolsController = (this.ctx as Record<string, any>).devToolsController; | ||
| if (devToolsController) { | ||
| logger.debug(TAG, 'reloadBridge via devToolsController RELOAD'); | ||
| devToolsController.eventEmitter.emit("RELOAD", { reason: 'HotReload2' }); | ||
| } else { | ||
| logger.debug(TAG, 'reloadBridge via restartAbility'); | ||
| await this.restartAbility(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== devTools reload hook usages =='
rg -n -C2 --glob '*.ts' --glob '*.ets' '\bdevToolsController\b|\beventEmitter\b|\bRELOAD\b|\bHotReload2\b' .
echo
echo '== UITurboModuleContext / related declarations =='
rg -n -C2 --glob '*.d.ts' --glob '*.ts' --glob '*.ets' '\b(interface|type|class)\s+UITurboModuleContext\b' .Repository: reactnativecn/react-native-update
Length of output: 1060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== PushyTurboModule relevant section =='
sed -n '1,220p' harmony/pushy/src/main/ets/PushyTurboModule.ts | cat -n
echo
echo '== Search for reloadBridge callers and restartAbility =='
rg -n -C3 'reloadBridge\(|restartAbility\(|reloadUpdate\(|restartApp\(' harmony/pushy/src/main/ets/PushyTurboModule.ts harmony/pushy/src/main/ets
echo
echo '== Search for devToolsController contract definitions/usages =='
rg -n -C3 --glob '*.ts' --glob '*.ets' --glob '*.d.ts' 'devToolsController|eventEmitter|RELOAD|HotReload2|UITurboModuleContext|TurboModuleContext' harmony .Repository: reactnativecn/react-native-update
Length of output: 24960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== all devToolsController references =='
rg -n -C3 --glob '*.ts' --glob '*.ets' --glob '*.d.ts' 'devToolsController' .
echo
echo '== all TurboModule context type references =='
rg -n -C3 --glob '*.ts' --glob '*.ets' --glob '*.d.ts' 'TurboModuleContext|UITurboModuleContext|ctx:' harmony
echo
echo '== PushyTurboModule file excerpt =='
sed -n '1,180p' harmony/pushy/src/main/ets/PushyTurboModule.ts | cat -nRepository: reactnativecn/react-native-update
Length of output: 10099
Guard the RELOAD path with the restart fallback. If eventEmitter is missing or emit('RELOAD', ...) throws, reloadUpdate() and restartApp() reject instead of falling back to restartAbility().
🤖 Prompt for 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.
In `@harmony/pushy/src/main/ets/PushyTurboModule.ts` around lines 68 - 75, The
reloadBridge path currently assumes
devToolsController.eventEmitter.emit("RELOAD", ...) will always succeed, so
reloadUpdate() and restartApp() can fail instead of using the restartAbility
fallback. Update reloadBridge() to guard the RELOAD branch in PushyTurboModule
by checking that eventEmitter exists and wrapping the emit call in error
handling; if emit is unavailable or throws, fall back to restartAbility() just
like the else path. Keep the logic localized to reloadBridge() so callers like
reloadUpdate() and restartApp() continue to resolve through the fallback.
1 participant
This PR aligns the HarmonyOS reload behavior with iOS/Android by implementing an in-memory soft-reload mechanism:
Changes:
reloadBridge()which emits the"RELOAD"event on RNOH'sdevToolsControllerfor a seamless, in-memory refresh (no window blinking). Falls back to the hard restart of the Ability if DevTools is unavailable.reloadUpdate()andrestartApp()to invokereloadBridge().Unit tests pass and HAR compiles successfully.
Summary by CodeRabbit
New Features
Bug Fixes