Android: preserve PDF callback events under Fabric , disable coalescing for shared PDF events #1011
Open
mavrickdeveloper wants to merge 1 commit intowonday:masterfrom
Conversation
TopChangeEvent coalescing so loadComplete is not dropped under Fabric
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.
Summary
This change makes Android
TopChangeEventnon-coalescible inreact-native-pdf.fixing : #1009
Problem
PdfView.loadComplete(...)andPdfView.onPageChanged(...)both dispatch through the sameTopChangeEvent/topChangepath.Under Fabric,
TopChangeEventinherits React Native's default coalescing behavior. Because both callbacks share the same event type for the same view, a laterpageChangedevent can replace an earlierloadCompleteevent before JS receives it.The result is that native Android successfully loads the PDF, but JS sometimes never receives
onLoadComplete.Root cause
PdfView.javadispatches bothloadComplete|...andpageChanged|...throughTopChangeEventTopChangeEvent.javaalways uses the same event name,topChangefabric/RNPDFPdfNativeComponent.jsexposes one bubbling event prop,onChangeTopChangeEventdoes not overridecanCoalesce(), so Fabric may merge these eventsFix
Override
canCoalesce()inTopChangeEventand returnfalse.That preserves both native callbacks in JS without changing the JS API or adding an app-level workaround.
Why this approach
onPageChangedas a substitute foronLoadCompleteTesting
Validated locally with temporary tracing:
loadCompletepageChangedpageChangedloadCompletepageChangedThis also restored the downstream flow that depended on
onLoadCompletefiring reliably.Notes
If team prefer, I can also follow this up with a larger refactor that splits these callback types into distinct Fabric/native events.
Fixes #1009