fix: keep the parser registration alive across effect remounts - #776
Draft
dariusz-biela wants to merge 3 commits into
Draft
fix: keep the parser registration alive across effect remounts#776dariusz-biela wants to merge 3 commits into
dariusz-biela wants to merge 3 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
MarkdownTextInput registered its parser worklet from a useMemo and only unregistered it from an effect cleanup. Whenever React remounted the effects without unmounting the component (StrictMode in development, a hidden React <Activity> that is revealed again), the cleanup erased the entry from the C++ registry while the decorator view kept the same parserId and the re-run of the effect registered nothing. Every later parse resolved the id with std::unordered_map::at and threw: iOS caught std::out_of_range and returned no ranges, Android let it cross the JNI boundary where fbjni turns it into a Java exception that MarkdownParser.java swallows. The input silently stopped formatting markdown for the rest of its life. Registration and unregistration now live in one layout effect and the new id reaches the decorator view through state. The first registration stays in the first render, so a mount still carries a resolvable id in its first commit and does not pay a second commit and a re-measure of the input. The effect body also replaces a registration whose parser changed identity while the effects were not mounted (an input inside a hidden <Activity>) and unregisters the stale one. A layout effect keeps the re-registration in the same task as the commit that ran the cleanup, so the two commits usually collapse into one native transaction instead of leaving the view on the erased id for a frame. No native change is needed: both parsers already return no ranges for an id the registry cannot resolve, and the ranges are cached per (text, parserId), so the replacement id re-parses the same text as soon as the view receives it. The new Jest suite renders the component through react-dom into jsdom and covers mount, unmount, StrictMode, a hidden and revealed <Activity>, a parser identity change inside a hidden <Activity> and a plain parser identity change; @types/react-dom is added so the suite typechecks.
dariusz-biela
force-pushed
the
fix/parser-registration-effect-remount
branch
from
September 8, 2026 15:29
06ea942 to
b2c6747
Compare
Author
|
I have read the CLA Document and I hereby sign the CLA |
…sters it JS unregisters the parser id when React cleans up effects, which also happens for an input that is hidden but still mounted. The native parser now looks the worklet up once, when the id prop changes, and keeps it alive for as long as the view lives, so a parse in that window still formats markdown. The registry is only a handoff from JS to native. On iOS `MarkdownParser` holds the worklet and both `RCTMarkdownUtils` paths pass the id through. On Android `MarkdownParser` becomes an fbjni hybrid that holds it, and the decorator view owns the parser so the `MarkdownUtils` recreated on every attach does not drop it. Claude-Session: https://claude.ai/code/session_01F1MRNtwY27QsvZcV1GwQJ9
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
1 participant
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.
Details
MarkdownTextInput(native) registered its parser worklet in auseMemoand unregistered it in auseEffectcleanup. When React ran the cleanup without unmounting the component (React StrictMode, a hidden<Activity>), the id was erased from the C++ registry,useMemodid not recompute, and the decorator view kept a danglingparserId. The native parser then returned no ranges, so the input silently stopped formatting markdown for the rest of its life.In the app
<Activity>is hidden and revealed, and under StrictMode in development.In the code
useParserIdhook. The body registers, the cleanup unregisters, and the fresh id reaches the decorator view through state.@types/react-domis added as a devDependency for the new test suite.Related Issues
Expensify/App#98254
Manual Tests
Added
src/__tests__/parserRegistration.test.tsx. It renders the nativeMarkdownTextInputthroughreact-dominto jsdom with a mocked registry and covers mount, unmount, StrictMode, a hidden and revealed<Activity>, and a parser identity change (visible and inside a hidden<Activity>). The StrictMode and<Activity>cases fail without the fix.To reproduce in the example app, wrap
MarkdownTextInputin<React.StrictMode>or in<Activity>and togglemode, then type*bold*. Before this change the text stays unformatted, after it the markdown is styled.Linked PRs
Expensify/App#100318