test: Cover View beforeUpdate/afterUpdate transaction semantics - #1513
test: Cover View beforeUpdate/afterUpdate transaction semantics#1513mrousavy wants to merge 2 commits into
Conversation
TestView now counts beforeUpdate()/afterUpdate() invocations, and a new Harness test asserts they bracket exactly the transactions that deliver at least one Nitro prop: - a mount with zero provided Nitro props is not a props transaction - a style-only resize is not a props transaction - a changed Nitro prop is exactly one transaction iOS already behaves this way (hasTransactionPropChanges gate). Android currently calls beforeUpdate()/afterUpdate() unconditionally on every updateState(), so this test is expected to be red on the Android Harness until the Kotlin ViewManager gates the calls the same way.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c45d77269
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| expect(view.getBeforeUpdateCount()).toBe(1) | ||
| expect(view.getAfterUpdateCount()).toBe(1) |
There was a problem hiding this comment.
On Android, the initial zero-prop mount and the following rerender both pass through HybridTestViewManager.updateState(), which unconditionally invokes beforeUpdate() and afterUpdate() at lines 57–59. Consequently, when this assertion runs the counters are already 2 rather than 1, so the Harness Android job that executes this test via .github/workflows/harness-android.yml will fail on every run. Gate the Android lifecycle calls before landing this assertion, or make the test reflect the behavior supported by each platform.
Useful? React with 👍 / 👎.
1 participant
Summary
TestViewnow countsbeforeUpdate()/afterUpdate()invocations (getBeforeUpdateCount()/getAfterUpdateCount()), keeping all existing required props requiredWhy
#1510 gates iOS prop delivery on
hasAnyProvidedProps()/hasSameProps(), sobeforeUpdate()/afterUpdate()only fire when a transaction actually delivers Nitro props - matching their doc contract ("Called right before updating props. React props are updated in a single batch/transaction.").Android's generated
ViewManager.updateState()still callsbeforeUpdate()/afterUpdate()unconditionally on every state update, including zero-prop mounts and style-only rerenders. That platform asymmetry was previously invisible because no fixture counted these calls.Expected CI status
🔴 This test is intentionally red on Harness Android until the Kotlin/JNI updater gates the calls the same way iOS does (e.g. by moving the
beforeUpdate/afterUpdatecalls into C++ behind the same provided/changed checks). iOS behavior already matches the asserted contract.Validation
🤖 Generated with Claude Code