Repository navigation
fix: Treat null as undefined for removed optional View props - #1515
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
mrousavy
force-pushed
the
fix/view-optional-prop-removal
branch
from
August 21, 2026 10:11
737d1c7 to
089bc77
Compare
mfazekas
added a commit
to rive-app/rive-nitro-react-native
that referenced
this pull request
Aug 23, 2026
Generated updateProps now diffs the new Props snapshot against the old one instead of consuming isDirty flags that live on the shared Props object (margelo/nitro#1503, #1506, #1510), so a component view Fabric recreates from an unchanged ShadowNode gets every provided prop applied again rather than nothing. That is what view-recreate.harness.tsx was written for. The peer range moves with it, so consumers have to bump too. Prop parsing also moved into Nitro core, which leaves the post-process shim for margelo/nitro#1184 without a parse site to patch — but that bug is still live, so clearing an optional prop throws mid-commit ("RiveView. layoutScaleFactor: Value is null, expected a number"). Carry mrousavy's own fix, margelo/nitro#1515, as a yarn patch until it ships, and cover both the optional<double> and the variant shape with a harness test. The generated EventPropertiesOutput variant is now ordered `boolean | number | string`, so the two hand-written helpers swap .second and .third. Both example apps make glog non-modular in post_install: 0.37.0 exposes React's renderer headers in the NitroModules modulemap and they reach <glog/logging.h>, whose headers include from inside `namespace google` — illegal once glog is a module, so every target that builds the NitroModules module fails to compile. Nothing in React Native imports glog as a module.
mfazekas
added a commit
to rive-app/rive-nitro-react-native
that referenced
this pull request
Sep 10, 2026
…380) Fixes #378. React Native 0.87 only calls the two-argument `installJSIBindingsWithRuntime:callInvoker:` TurboModule hook, which nitro added in 0.36.2. With the previous `<0.36` peer range no Nitro module could install and the app never registered. Reproduced in a bare RN 0.87.0 app with the published 0.5.0-beta.3: native build succeeds, blank screen, `Failed to install Nitro: installJSIBindingsWithRuntime: was not called` in the log. This moves nitrogen and `react-native-nitro-modules` to 0.36.5, regenerates the specs and widens the peer range to `>=0.35.10 <0.37`. The floor is unchanged because the regenerated specs still build and run against nitro 0.35.10 (verified on both platforms plus three harness suites), so existing consumers are not forced to bump; RN 0.87 users need 0.36.2 or newer, which the README now says. The cap stays below 0.37 on purpose: nitro 0.37 moves prop parsing into its core, where clearing an optional prop still throws (margelo/nitro#1515 is unmerged), and our existing null-prop shim only anchors on the 0.36 generated code. nitrogen 0.36 changes two things the generated view code relies on. Both are handled in `scripts/nitrogen-postprocess.ts`, the same way nitro 0.37 fixes them upstream: 1. The component descriptor is built with a plain `RawPropsParser()`, which only keeps props as `jsi::Value`s on RN >= 0.85. Older versions convert every prop to `folly::dynamic` and the generated Props constructor aborts on the first RiveView mount. The postprocess forces the JSI parser where the flag still exists. 2. The component view never sets `_props`, which RN 0.87 turns into a fatal `RCTViewComponentView` assertion on debug builds. The postprocess initializes it to the component's default props in `init`. The `EventPropertiesOutput` variant is now ordered `boolean | number | string`, so the hand-written Swift and Kotlin helpers swap their cases. Verified: example iOS (RN 0.80.3) harness 28 suites / 208 tests, Android harness 28 suites / 208 tests, and a bare RN 0.87.0 app with the packed tarball + nitro 0.36.5 installs Nitro and renders `rewards.riv`. The same bump should be backported to release-v0.4 so npm `latest` works on RN 0.87 too.
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
Throwaway spike for #3732, not for merging. apps/mobile/modules/stim-ticker is a local Nitro package linked with link:./modules/stim-ticker. A Ticker HybridObject (Swift, Kotlin) calls a native consumer at about 30 Hz from a background queue or HandlerThread. TickerView (Swift, Kotlin) takes ticker as a prop, registers itself as the consumer through a native-only API and hops to main to draw the count. The temporary route stim://ticker-spike exercises swap, detach, unmount, re-render and a blocked JS thread. The nitro-modules patch backports margelo/nitro#1515: React sends null for a removed prop, and 0.37.1 throws converting null to an optional HybridObject, see margelo/nitro#1184. Refs #3732
4 tasks done
This was referenced Oct 14, 2026
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
janicduplessis
added a commit
to appandflow/stim
that referenced
this pull request
Oct 14, 2026
…mits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs #3735
Andarius
added a commit
to Obitrain/obi-image
that referenced
this pull request
Oct 19, 2026
Fabric sends null for a removed prop and Nitro rejects it for optional props. Key the native view on prop presence so it remounts when a prop goes from set to unset. Drop once margelo/nitro#1515 ships.
Andarius
added a commit
to Obitrain/obi-image
that referenced
this pull request
Oct 19, 2026
…ved (#39) * fix: stop crashing when an image's source, tint or callbacks are removed Fabric clears a removed prop by sending null, which Nitro rejects for optional props ("ObitrainImageView.uri: Value is null, expected a String") and kills the app, e.g. when obiapp's avatar falls back to initials after a load error. The wrapper now always sends every prop ('' / 0 / no-op mean none) and native treats an empty uri and a 0 tint as none. The example's Demo gains a clear-source case. * fix: remount the native view instead of sending sentinel props Fabric sends null for a removed prop and Nitro rejects it for optional props. Key the native view on prop presence so it remounts when a prop goes from set to unset. Drop once margelo/nitro#1515 ships.
zuxine
pushed a commit
to zuxine/stim
that referenced
this pull request
Nov 4, 2026
…o view (appandflow#3767) * fix(mobile): patch Nitro for removed object props and raise socket limits Backports margelo/nitro#1515 so a removed hybrid object prop arrives as undefined, and lets both react-native-nitro-websockets transports carry a 16 MiB message plus the Noise type byte and tag. The Android build also publishes the generated headers as a prefab package, so the session module can link against the socket's HybridObject. Refs appandflow#3735 * feat(mobile): add the native Noise session and Nitro video view apps/mobile/modules/stim-session is a linked Nitro package. Its C++ StimSession wraps a react-native-nitro-websockets socket, runs the Noise XXpsk2 and XX handshakes and Stim's framing on vendored libsodium 1.0.22, keeps the client key in the Keychain or behind an Android Keystore key, and hands only plaintext RPC to JavaScript. Video messages go to a VideoStream that the StimVideoView decodes with the VideoToolbox and MediaCodec code moved from the Expo StimVideo module, which this removes. Refs appandflow#3735 * feat(core): describe the protocol 2 pairing code and hello for the phone Refs appandflow#3735 * feat(mobile): serve protocol 2 from the mock server The mock server answers the Noise handshake with @stim-cli/core/noise, prints a v2 pairing code with its server key, and dev:pair writes that code for a development build to spend. Refs appandflow#3735 * feat(mobile): connect to stim-server with protocol 2 only The connection runs over the native session, pins the server key from pairing, drops protocol 1 pairings, accepts ws:// to private addresses, and names an old stim-server when it refuses the Noise prefix. Device views pass the subscription's VideoStream to the native view. Refs appandflow#3735 * feat(mobile): add native timing, a vector test and protocol 2 docs Development builds aggregate the session's decrypt and route times and each VideoStream's decode-queue and present times and dropped frames, readable through timing() and shown in the stream overlay. A node test compiles cpp/Noise.cpp with the vendored libsodium and checks it against packages/core's Noise and framing vectors, and stim://noise-check measures the video path on a device. The README describes protocol 2 pairing, the new module and dev:pair. Refs appandflow#3735 * fix(mobile): keep libsodium's headers from resolving to other pods' Refs appandflow#3735 * perf(mobile): compile libsodium with optimizations in every configuration Refs appandflow#3735 * fix(mobile): keep a stream's first keyframe and report protocol 2 in About The session keeps the video messages of a subscription from its last keyframe until JavaScript asks for its stream, and a stream reports its current format when JavaScript subscribes to it, so a view opens on the first keyframe instead of asking for another. The website's phone page describes protocol 2 pairing. Refs appandflow#3735 * feat(mobile): follow stim-server's protocol 2 pairing The QR code carries the server key's fingerprint, which the native session checks against the key the server proves before it sends message 3. Message 3 carries the device name as {"name": ...}, as stim-server reads it, and hello sends only the client. The phone protocol uses the server's PROTOCOL_VERSION 2 and HelloParams, the mock server authenticates with @stim-cli/core/noise-session instead of its own copy, and the spec's release order has the phone on protocol 2 only. Refs appandflow#3735 * build(core): regenerate the phone RPC validators for the protocol 2 hello and pairing code Refs appandflow#3735 # Conflicts: # packages/core/receive-validators.mjs # Conflicts: # packages/core/receive-validators.mjs * fix(mobile): let the banner name the remedy for a refused connection Refs appandflow#3735 * fix(mobile): keep native video streams ordered and thread safe A VideoStream now hands frames to its view and replays its backlog with its lock held, so a frame pushed during attach cannot reach the decoder before the buffered keyframe. Each frame names its stream, and the view decodes only frames of the stream its decode queue last attached, so the socket thread no longer reads the view's attached stream while the UI thread swaps it, and a frame copied before a detach is dropped. The format and progress listeners are stored and read under the lock and cleared on dispose, which also breaks the cycle through the stream's JavaScript object. Refs appandflow#3735 * perf(mobile): attach the socket thread to the JVM once on Android fbjni's ThreadScope attached and detached the socket thread around every video message. withJvm now attaches a thread the JVM does not know once, runs its first call with the app class loader so fbjni caches the Kotlin classes, calls directly afterwards, and detaches the thread when it exits. Threads Java started keep the class loader on every call. Refs appandflow#3735 * fix(mobile): recover from a client key the phone cannot read Android Auto Backup and device transfer carried stim.noise.xml but not the Keystore key that wraps it, so after a restore or reinstall every connection and pairing failed. A config plugin now points both backup rules at files that exclude it as well as SecureStore. ClientKeyStore discards an entry it can never read, so the session makes a new key, and the Mac list drops its Macs when the client key changed, so the phone pairs again. Index reads and writes run one at a time and a migration that cannot write is reported instead of returning no Macs. The handshake also wipes its PSK copy and the HKDF temporaries. Refs appandflow#3735 * fix(mobile): retry a failed hello and clear disposed video streams A hello that could not be sent, or failed without a refusal, closed the socket, whose close the session no longer reports, so the connection stayed connecting. It now detaches and schedules a retry, and a request whose send throws leaves no pending entry. The connection tells the subscriber before it disposes a stream, and the device stream hook ignores callbacks from a stream it no longer shows. A client key the phone cannot read gets its own message instead of Connection lost, and the overlay shows the route time. Refs appandflow#3735 * build(mobile): fetch the pinned libsodium release instead of vendoring it The repository no longer holds libsodium. libsodium.json pins the official 1.0.22 release tarball by SHA-256, with its GitHub release as a mirror, and scripts/fetch-libsodium.mjs downloads it into a per-user cache, checks the hash, and extracts only its C sources and headers into the gitignored third_party/libsodium through a temporary folder renamed into place. The podspec runs it when CocoaPods reads it, CMake when it configures, and the vector test before it compiles, so every build uses the same release. The fingerprint, Metro and watchman ignore the extracted folder, and the mobile workflow caches it. Refs appandflow#3735
React Fabric sends null when a prop is removed. Convert it to undefined for optional props whose types reject null, and skip callback unwrapping for cleared values. Preserve explicit null values for nullable types and validation for required props. Keep the view regression test compatible with the current TestView props.
mrousavy
force-pushed
the
fix/view-optional-prop-removal
branch
from
November 9, 2026 20:45
089bc77 to
9a30232
Compare
This branch was previously deployed
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.
undefinedto optional view property #1184Summary
nullsentinel for a removed prop (ReactNativeAttributePayload: "Flag the leaf property for removal by sending a sentinel") - andprop={undefined}takes the same path, so the two are indistinguishable on the wireCachedProppreviously passed thatnullstraight into conversion, which throws mid-commit for every optional prop type that rejects null (optional<double>viaasNumber(), andhybridRefeven earlier, in the{ f }function-prop unwrap)nullis not a valid value for it (JSIConverter<T>::canConvert), the value is normalized toundefined→ the native setter is called once withnulloptSemantics
undefined→ setter receivesnullopt(the JS-visible value becomesundefined). The native initializer default still only covers never-provided props - a richer "reset on clear" belongs in the implementation (storage ?? default)null(NullType, variants containing null): untouched -nullstays data, so removal collapses to "explicit null" for those (inherent to React's sentinel protocol)ViewName.propName:prefixBecause normalization only applies where conversion was guaranteed to throw, no currently-working behavior can change.
Overhead
if constexpr (is_optional<T>)branch compiles outvalue.isNull()tag check during Props parsing; thecanConvertwalk only runs when the value actually isnull(i.e. an actual removal)Testing
New Harness test
delivers undefined when an optional prop is removed": mounts TestView withnativeDefaultValue={1}+hybridRef, then removes both - asserts the value readsundefined, the setter fired exactly once more, unrelated setters (isBlue) did not fire, and the previously-crashinghybridRef` removal path survives. Red without the fix (commit-time throw), green with it.Validated: workspace typecheck, eslint (CI settings), clang-format.
🤖 Generated with Claude Code