Attempting a fix for inconsistency in text gradient clamp logic between iOS and Android.#57588
Closed
andrewkunkel wants to merge 363 commits into
Closed
Attempting a fix for inconsistency in text gradient clamp logic between iOS and Android.#57588andrewkunkel wants to merge 363 commits into
andrewkunkel wants to merge 363 commits into
Conversation
New arch version of: 107d9a8
## Issue During app startup, react-native's `AppState.currentState` can incorrectly report `background` when the app is actually in the foreground. This happens because: 1. `AppStateModule` relies on `onHostResume` and `onHostPause` event subscriptions to track state 2. The initial state can incorrectly initialize to `APP_STATE_BACKGROUND` when `reactContext.lifecycleState === LifecycleState.BEFORE_CREATE` (which occurs before root view attachment) ## Solution This PR adds an `isAppForegroundedByMemoryState()` method that provides a more accurate initial state detection when the React context is in `BEFORE_CREATE` lifecycle state. ## Future Improvements While this is a quick workaround to address the immediate issue, potential future improvements: 1. Leveraging Android activity lifecycle to set initial state to `RESUMED` on `onCreate` 2. Mapping `LifecycleState.BEFORE_CREATE` to `unknown` instead of incorrectly mapping to `background`
…hy on UI thread (react#66) * fix(new arch): change measure function to measure native view hierarchy on UI thread * fix ios compile errors * add second parameter `measureOnUI` to .measure function which we only set to true for pressability
Co-authored-by: Hanno J. Gödecke <die.drei99@yahoo.de>
Create UIManagerConstantsCache Update UIManagerModule.java Update ReactActivity.java Update UIManagerConstantsCache Update UIManagerModule.java Update JavaModuleWrapper.cpp Update and rename JavaModuleWrapper.kt to JavaModuleWrapper.java Rename UIManagerConstantsCache to UIManagerConstantsCache.java
… Reanimated stalls React-Freeze’s infiniteThenable causes Fabric’s RuntimeScheduler_Legacy to spin indefinitely within the while loop within startWorkLoop, which prevents the JS thread from ever flushing its microtask queue (handled by JSTimers/MessageQueue). When the queue starves, Reanimated worklets never run and animations freeze. This patch adds an explicit call to performMicrotaskCheckpoint(runtime) immediately after each task execution inside startWorkLoop. The helper invokes the global _flushReactNativeMicrotasks JS function (registered from JSTimers) so the queued microtasks are processed even while the native work-loop is busy. ✓ Restores Reanimated animations under Fabric bridgeless ticket: https://app.asana.com/1/236888843494340/project/1199705967702853/task/1210188774106277?focus=true
…act#93) THis can be reverted once we found a upstream solution with meta here: react#52919
…location
Guard showOrUpdate() to take the dialog-creation path when dialog is null, preventing checkNotNull(dialog) crash in updateProperties(). Fabric's PreAllocateViewMountItem can call onAfterUpdateTransaction before any property setter fires createNewDialog = true, leaving both dialog = null and createNewDialog = false — the else branch then crashes.
java.lang.IllegalStateException: dialog must exist when we call updateProperties
at com.facebook.react.views.modal.ReactModalHostView.updateProperties(ReactModalHostView.kt:376)
at com.facebook.react.views.modal.ReactModalHostView.showOrUpdate(ReactModalHostView.kt:261)
at com.facebook.react.views.modal.ReactModalHostManager.onAfterUpdateTransaction(ReactModalHostManager.kt:120)
at com.facebook.react.views.modal.ReactModalHostManager.onAfterUpdateTransaction(ReactModalHostManager.kt:25)
at com.facebook.react.uimanager.ViewManager.updateProperties(ViewManager.java:105)
at com.facebook.react.uimanager.ViewManager.createViewInstance(ViewManager.java:221)
at com.facebook.react.uimanager.ViewManager.createView(ViewManager.java:148)
at com.facebook.react.fabric.mounting.SurfaceMountingManager.createViewUnsafe(SurfaceMountingManager.java:766)
at com.facebook.react.fabric.mounting.SurfaceMountingManager.preallocateView(SurfaceMountingManager.java:1212)
at com.facebook.react.fabric.mounting.mountitems.PreAllocateViewMountItem.execute(PreAllocateViewMountItem.kt:38)
at com.facebook.react.fabric.mounting.MountItemDispatcher.executeOrEnqueue(MountItemDispatcher.kt:312)
at com.facebook.react.fabric.mounting.MountItemDispatcher.dispatchPreMountItemsImpl(MountItemDispatcher.kt:290)
at com.facebook.react.fabric.mounting.MountItemDispatcher.dispatchPreMountItems(MountItemDispatcher.kt:269)
at com.facebook.react.fabric.FabricUIManager$DispatchUIFrameCallback.doFrameGuarded(FabricUIManager.java:1518)
at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.kt:25)
at com.facebook.react.modules.core.ReactChoreographer.frameCallback$lambda$1(ReactChoreographer.kt:59)
* Port observer-lifetime fix for image-recycle crash (react#55135) Backport of upstream PR react#55135 ("fix rctimage observer crash when unloading with events in-flight"). Convert ImageResponseObserverCoordinator's observers from raw pointers to std::shared_ptr<const ImageResponseObserver> so an observer cannot dangle while an image event is in flight (the documented react#55135 crash in nativeImageResponseProgress). RCTImageComponentView now holds its proxy as a shared_ptr and passes it directly (a final ImageResponseObserver subclass converts implicitly to shared_ptr<const ImageResponseObserver>). Also fix an over-removal bug in removeObserver: erase(position, end()) removed every observer from the match onward, which could prematurely empty observers_ and trigger an unwanted cancelRequest_(). Use erase(position) to remove only the single matched observer, matching the add/remove balance the comment describes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Guard nil cancellation block in RCTImageManager to fix image-recycle crash `loaderRequest.cancellationBlock` can be nil (a loader that returns no cancellation block, or a request that has already completed). The captured `sharedCancelationFunction` invoked it unconditionally, so when a cancel is triggered during view recycling (RCTImageComponentView::prepareForRecycle → _setStateAndResubscribeImageResponseObserver → removeObserver → cancelRequest_()), a nil block was called. Invoking a nil Objective-C block dereferences `block->invoke` at offset 0x10 → EXC_BAD_ACCESS / KERN_INVALID_ADDRESS at 0x10. Nil-guard the captured block so the cancel is a no-op when there is nothing to cancel. This is Discord-specific hardening on top of the upstream observer-lifetime fix (react#55135, ported in the preceding commit) and is the likely direct cause of the reported crash. Fixes DISCORD-IOS-M4EH Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * remove redundant comments --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On the New Architecture (Fabric), Reanimated commits the shadow tree ~once per animation frame, and each mount transaction unconditionally called _remountChildren, re-clipping the entire descendant subtree even when nothing scrolled or changed layout. Guard _remountChildren with RCTMountingTransactionAffectsClipping, which returns YES only when the transaction can change clipping: an Insert/Remove mutation, an Update with changed layoutMetrics, or an Update toggling removeClippedSubviews. maintainVisibleContentPosition is unaffected since _adjustForMaintainVisibleContentPosition still runs on every mount.
* fix(ios): rendering hitches * avoid setting overflow twice for Legacy Interop components * Revert "avoid setting overflow twice for Legacy Interop components" This reverts commit 66ed378.
… RCTApplyLayerCornerConfiguration
…on to correctly support non-horizontal angles. The intent of the gradientWidth prop was to define the width of the gradient itself, but it was poorly named on the assumption the gradient would be horizontal. This led to some confusion in the Android implementation, so it incorrectly references the text width, which breaks the vertical gradient measurement. This PR renames the prop for clarity, and fixes the calculations. When the client switches to this version, we'll rename the prop in UserNameWithEffects, and begin passing in the height of the text as its value. Currently gradientWidth is not being passed at all for the Toon prop. Before: <img width="572" height="93" alt="image" src="https://github.com/user-attachments/assets/44d873fd-dc67-4622-acf9-33fbfcfbcc3a" /> After: <img width="226" height="51" alt="image" src="https://github.com/user-attachments/assets/4fa5816b-2a70-4f81-ad30-e74a39213396" />
…d, rather than abruptly looping the gradient.
|
Hi @andrewkunkel! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Warning Missing Test Plan Please add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested. |
|
Warning JavaScript API change detected This PR commits an update to
This change was flagged as: |
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
20 participants
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.
In the
gradientModeprop we added to text gradient, there's a platform-specific discrepancy between how it interacts withgradientLengthwhen the provided length is too short:This is an existing discrepancy which we didn't introduce in our code. Android natively supports both clamp and mirror modes, while on iOS clamp is the only mode and we had to add support for mirrored text gradients.