Skip to content

[HOLD for RN 0.88] fix: keep the parser registration alive across effect remounts - #776

Draft
dariusz-biela wants to merge 10 commits into
Expensify:mainfrom
dariusz-biela:fix/parser-registration-effect-remount
Draft

dariusz-biela wants to merge 10 commits into
Expensify:mainfrom
dariusz-biela:fix/parser-registration-effect-remount

Conversation

@dariusz-biela

@dariusz-biela dariusz-biela commented Sep 8, 2026

Copy link
Copy Markdown

Important

On hold until React Native 0.88. The Fabric renderer in React Native 0.85–0.87 does not run the useInsertionEffect cleanup for a component removed while its <Activity> is hidden (see "Native renderer caveat"). React 19.3, which React Native 0.88 ships, always runs it. The diff itself does not need to change.

Details

MarkdownTextInput registered its parser worklet during render and unregistered it in an effect cleanup. React can run that cleanup without unmounting the component (StrictMode, a hidden <Activity>), after which the input silently stopped formatting markdown.

The fix

The parser id is now chosen in JS, one per mounted input and a fresh one on every parser change, and the worklet is registered in useInsertionEffect inside the new useParserId hook. Insertion effects are neither double-invoked in StrictMode nor disconnected for a hidden <Activity>, and they run before the host tree is committed, so the registration lives exactly as long as the input and the first commit already carries a valid id.

The native registry becomes a plain map from id to worklet; an unknown id yields no ranges instead of throwing.

Native renderer caveat

React Native 0.85–0.87 build the Fabric renderer with enableHiddenSubtreeInsertionEffectCleanup off, so removing an input behind a hidden <Activity> skips the insertion cleanup and leaks its worklet in the native registry. Formatting stays correct because ids are never reused. react-dom has the flag on, and React 19.3 removed it and always runs the cleanup (react/react#30954, #34372, #35918).

Example app

New buttons reproduce the scenarios on a device: hide and show an <Activity> around the input, keep the input painted while hidden (AlwaysPaintedView), and swap the parser prop.

Related Issues

Expensify/App#98254

Manual Tests

src/__tests__/parserRegistration.test.tsx covers mount, unmount, StrictMode, hidden and removed <Activity>, an abandoned render, shared parsers and a parser swap, and checks that nothing stays registered. The StrictMode and <Activity> cases fail without the fix.

Example app on iOS and Android with rebuilt native code:

  1. Cold start: the initial value is formatted on the first frame.
  2. Type Hello *bold* and https://expensify.com: bold and link render as you type.
  3. Hide and show the <Activity>: the input comes back formatted and keeps formatting new text.
  4. Swap the parser back and forth: the input follows each swap.
  5. Wrap the input in <React.StrictMode> and type *sm*: it renders bold.
iOS Android
ios-activity-buttons.mp4
android-activity-buttons.mp4

Linked PRs

Expensify/App#100318

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

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
dariusz-biela force-pushed the fix/parser-registration-effect-remount branch from 06ea942 to b2c6747 Compare September 8, 2026 15:29
@dariusz-biela

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

exfy-clabot Bot added a commit to Expensify/CLA that referenced this pull request Sep 8, 2026
…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
Registering in render leaked an entry whenever React abandoned the
render (a suspended tree, an <Activity> removed while still hidden), and
the extra bookkeeping in useParserId existed only to pair that render
registration with the effect. The layout effect now registers, its
cleanup unregisters, and the id reaches the decorator through state. The
first commit carries 0, which both native parsers treat as no parser;
on iOS parseUncached resolves the worklet before touching the worklet
runtime, since that first commit can arrive before the runtime exists.

The jsdom suite gains cases for a same-parser rerender, an Activity that
is removed while hidden, an abandoned suspended render, a replacement
parser in a previously visible Activity, and two inputs sharing one
parser, and asserts that every test leaves the registry empty.
The parser id is now picked in JS, one per worklet identity, and the worklet
is registered in `useInsertionEffect`. That effect runs before the host tree
commits, so the first measure already sees the worklet, and a hidden
`<Activity>` leaves it connected, so the id the native view holds never goes
stale. Inputs sharing a parser share one registration through a retain count.

This removes the extra render and commit of the layout effect approach and
the native keep-alive of the worklet, which is no longer needed. The C++
registry accepts the id from JS and returns nullptr for unknown ids instead
of throwing.
Two buttons hide and reveal the input in a React Activity, one with a plain
tree and one with AlwaysPaintedView, a view whose display style is pinned to
contents so the input stays painted while its JS side is hidden. A third
button swaps the parser prop between ExpensiMark and a strikethrough worklet.
The example passes an explicit max length to parseExpensiMark because the
default parameter is evaluated before the worklet closure with
react-native-worklets 0.10.2.
Inputs no longer share a registration per parser function. Each mounted input
allocates its own id, and a fresh one whenever the parser prop changes, so the
native parse cache keyed by text and parser id never serves ranges from a
previous parser. This removes the parser-to-id WeakMap and the retain count.
…Activity buttons

The example gets one button that picks the wrapper while the input is visible
and one that hides or shows the Activity, instead of a hide button per wrapper.
@dariusz-biela dariusz-biela changed the title fix: keep the parser registration alive across effect remounts [HOLD for RN 0.88] fix: keep the parser registration alive across effect remounts Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant