fix(harmony): align isFirstTime, rollback, and soft reload with android/ios - #587
fix(harmony): align isFirstTime, rollback, and soft reload with android/ios#587sunnylqm wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesFirst-load marker, ignoreRollback, and reload bridge
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
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 » harmony/pushy/src/main/ets/UpdateContext.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/UpdateContext.ts`:
- Around line 402-408: `UpdateContext` is setting `firstLoadMarked` and
`ignoreRollback` too early, before `launchState.loadVersion` is confirmed and a
verified bundle is returned. Move the `persistState(...)` and
`UpdateContext.ignoreRollback = true` logic so it runs only after the selected
bundle file has been validated and is about to be returned, or explicitly clear
both flags on the missing-file rollback paths before calling `rollBack()`.
🪄 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: 00a31086-8fe6-42cc-b21c-eb0c6ea287ea
📒 Files selected for processing (3)
harmony/pushy/src/main/ets/PushyFileJSBundleProvider.etsharmony/pushy/src/main/ets/PushyTurboModule.tsharmony/pushy/src/main/ets/UpdateContext.ts
| if (launchState.didRollback || launchState.consumedFirstTime) { | ||
| this.persistState(launchState); | ||
| this.persistState(launchState, { | ||
| markFirstLoadMarker: launchState.consumedFirstTime, | ||
| }); | ||
| } | ||
| if (launchState.consumedFirstTime) { | ||
| UpdateContext.ignoreRollback = true; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only mark first-load after the selected bundle is verified.
firstLoadMarked and ignoreRollback are set before the loop confirms launchState.loadVersion exists. If that file is missing, the later rollback path can load a fallback bundle while getConstants() still reports isFirstTime: true for the failed version.
Suggested direction
- if (launchState.didRollback || launchState.consumedFirstTime) {
+ const shouldMarkFirstLoad = !!launchState.consumedFirstTime;
+ if (launchState.didRollback || shouldMarkFirstLoad) {
this.persistState(launchState, {
- markFirstLoadMarker: launchState.consumedFirstTime,
});
}
- if (launchState.consumedFirstTime) {
- UpdateContext.ignoreRollback = true;
- }Then set firstLoadMarked / ignoreRollback only immediately before returning a verified bundleFile, or clear both before calling rollBack() on the missing-file paths.
🤖 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/UpdateContext.ts` around lines 402 - 408,
`UpdateContext` is setting `firstLoadMarked` and `ignoreRollback` too early,
before `launchState.loadVersion` is confirmed and a verified bundle is returned.
Move the `persistState(...)` and `UpdateContext.ignoreRollback = true` logic so
it runs only after the selected bundle file has been validated and is about to
be returned, or explicitly clear both flags on the missing-file rollback paths
before calling `rollBack()`.
1 participant
This PR resolves critical design discrepancies in the HarmonyOS implementation, aligning it with the robust state-machine logic and reload experiences of the iOS and Android platforms:
1. isFirstTime Loading & Rollback Protection
firstTimevalue from preferences directly on React Native startup without ever consuming or resetting it. Furthermore, theconsume_first_time_on_launchparameter was hardcoded tofalse. This prevented rollback protection from triggering on crash/failure, and preventedisFirstTimefrom being a single-use flag.consume_first_time_on_launch = trueduringgetBundleUrl().firstLoadMarkedkey to Preferences when the launch state consumes first time.consumeFirstLoadMarker()to read and immediately delete the persistent marker.clearFirstTime()to deletefirstLoadMarked.ignoreRollbackas a static memory variable to handle multiple bundle path resolutions during a single application process startup.2. In-Memory Soft Reload
terminateSelf+startAbility), causing the window to blink/go white. Both Android and iOS support smooth in-memory soft reloading.reloadBridge()to emit the"RELOAD"event to RNOH'sdevToolsControllerfor seamless in-memory reloading.restartAbility()ifdevToolsControlleris unavailable.PushyFileJSBundleProvider.etsto dynamically fetch the update bundle URL on every call togetURL()/getBundle(), ensuring that when the React Native container reloads, it fetches the newly switched version.All JS/TS unit tests pass (70/70) and the Harmony HAR compiled successfully.
Summary by CodeRabbit
New Features
Bug Fixes