Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix new debugger detection after #864 #904

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

kmagiera
Copy link
Member

In #864 where we tried to address the debugger listing changes in RN 77 we accidently broke debugger connection on older versions of React Native (in particular expo-router example app).

The reason is that we wrongly assumed that the existence of reactNative field in the debugger listing is exclusive to the new debugger entries while it still appears with old version of the debugger and on older versions of React Native. As a consequence, older apps (and expo-router in particular) may select a wrong runtime to connect to (which in case of expo-router was the reanimated runtime).

In this PR we partially revert changes from #864 responsible for filtering new debugger entries from debug endpoints list. Instead, for RN 77 specifically we rely on a suffix "[C++ connection]" that has been added to description field: https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/jsinspector-modern/InspectorPackagerConnection.cpp#L167

We likely want to revisit this solution going forward, but for now this PR will unblock the release.

How Has This Been Tested:

  1. Test debugger on RN 77 example
  2. Test debugger on expo-router example

Copy link

vercel bot commented Jan 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
radon-ide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 11:00pm

@kmagiera kmagiera merged commit 3613b0d into main Jan 14, 2025
4 checks passed
@kmagiera kmagiera deleted the kmagiera/fix-debugger-after-864 branch January 14, 2025 23:01
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