Skip to content

Avoid deprecated iOS tag lookup for chart commands (PRO-4970) - #21

Merged
furkando merged 1 commit into
mainfrom
fix/ios-ref-command-dispatch
Sep 20, 2026
Merged

furkando merged 1 commit into
mainfrom
fix/ios-ref-command-dispatch

Conversation

@furkando

@furkando furkando commented Sep 20, 2026 •

Copy link
Copy Markdown

Summary

iOS still converts the chart ref to a numeric tag and calls UIManager.dispatchViewManagerCommand. On Fabric that goes through findShadowNodeByTag_DEPRECATED, which walks the whole shadow tree on the JS thread and can race a commit from another thread. #20 removed this path on Android only.

It crashed on iOS during chart testing of perps-fe-mobile #2236 (iPhone 16 Pro simulator, RN 0.86.3, debug build): EXC_BAD_ACCESS on com.facebook.react.runtime.JavaScript in ShadowNode::getTag <- findShadowNodeByTagRecursively <- UIManager::findShadowNodeByTag_DEPRECATED, called from a JS timer, while the main thread was inside Reanimated's worklet runtime. React Native 0.86 still ships the fix for that lookup behind fixFindShadowNodeByTagRaceCondition, off by default. The app issues about 400 chart commands in two minutes on an unauthenticated 1m chart, so the lookup runs constantly.

This PR uses codegenNativeCommands with the current host ref on iOS as well, so no chart command resolves a numeric tag. The iOS view is a legacy view manager behind Fabric's interop layer, which resolves commands by method name (methodsByName) and prepends the view tag itself, so the native RCT_EXTERN_METHOD signatures, command names, argument order and payloads are unchanged.

Android: no behavior change. Android already used this exact call (Commands[name](view, ...args)); the diff only removes the platform branch around it. No native code is touched on either platform.

This removes the chart's exposure to the racy lookup on iOS. It does not prove every occurrence of the intermittent crash has this cause; other libraries can still use numeric tags.

Linear ticket: PRO-4970

Validation

  • 29 command-dispatch tests pass. iOS now asserts the host-ref path for all 11 commands, and a new test per platform asserts that no command calls findNodeHandle or the legacy dispatcher.
  • iOS runtime check with this exact index.js patched into perps-fe-mobile 2db9f659f (Expo 57, RN 0.86.3, iPhone 16 Pro simulator, mainnet data): native KLine chart loads, appends new 1m candles and updates the live price; updateLastCandlestick, addOrderLine and removeOrderLine all executed; no "No command found" errors, no crash. About two minutes, so this shows the commands work, not that the crash is gone.
  • Still to do before release: a soak on iOS with the published prerelease (provider switch, background within 20 s, repeat; interval loops 3D -> 1W -> 1M; background/foreground) with crash logs, and the usual Android regression run since the package version changes.

Author Checklist

  • PR tested locally
  • PR tested on preview

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

✅ PR Checklist Validation Passed

Status: All 2 checklist items completed
Linear ticket: Valid ✅

🎉 This PR meets all requirements and is ready for review!


This validation ensures all PR requirements are met before merging.

@linear

linear Bot commented Sep 20, 2026

Copy link
Copy Markdown

PRO-4970

@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

🚀 Prerelease Published

Version: 0.21.0-furkando.285ddb9.fix-ios-ref-command-dispatch
Tag: fix-ios-ref-command-dispatch

Update package.json:

"@elliottech/react-native-kline-view": "0.21.0-furkando.285ddb9.fix-ios-ref-command-dispatch"

Previous prereleases for this PR have been cleaned up. This prerelease will be automatically cleaned up when the PR is merged.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@furkando
furkando merged commit aacadc0 into main Sep 20, 2026
4 of 5 checks passed
@github-actions

Copy link
Copy Markdown

🧹 Prerelease Cleanup Complete

All prerelease versions for branch fix-ios-ref-command-dispatch have been cleaned up.

The following actions were performed:

  • ✅ Unpublished all versions tagged with fix-ios-ref-command-dispatch
  • ✅ Removed prerelease packages from npm registry

This cleanup was performed automatically after PR merge.

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