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

Debugging JSContext in safari is not enabled for iOS >= 16.4 #39488

Closed
jefflewis opened this issue Sep 16, 2023 · 10 comments
Closed

Debugging JSContext in safari is not enabled for iOS >= 16.4 #39488

jefflewis opened this issue Sep 16, 2023 · 10 comments

Comments

@jefflewis
Copy link

jefflewis commented Sep 16, 2023

Description

As of iOS 16.4 Apple requires the isInspectable property of the JSContext to be set to true in order for the JSContext to be shown in the Safari inspector window, as documented as a debugging tool here.

This patch would apply this property to DEBUG builds and is what I have done to successfully get debugging working again.

If I open this as a pull request, would it be accepted?

patch:

--- a/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp
+++ b/node_modules/react-native/ReactCommon/jsc/JSCRuntime.cpp
@@ -409,12 +409,12 @@ JSCRuntime::~JSCRuntime() {
@@ -398,6 +398,11 @@ JSCRuntime::JSCRuntime(JSGlobalContextRef ctx)
       stringCounter_(0)
 #endif
 {
+#ifndef NDEBUG
+  if (__builtin_available(iOS 16.4, *)) {
+    JSGlobalContextSetInspectable(ctx, true);
+  }
+#endif
 }

Reference:

https://webkit.org/blog/13936/enabling-the-inspection-of-web-content-in-apps/

React Native Version

0.72

Output of npx react-native info

npx react-native info
warn Package @sentry/react-native contains invalid configuration: "dependency.platforms.ios.sharedLibraries" is not allowed,"dependency.hooks" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
warn Package rn-fetch-blob contains invalid configuration: "dependency.hooks" is not allowed. Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.
info Fetching system and libraries information...
System:
  OS: macOS 13.4.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 78.22 MB / 16.00 GB
  Shell:
    version: 3.6.1
    path: /opt/homebrew/bin/fish
Binaries:
  Node:
    version: 16.20.2
    path: ~/.asdf/installs/nodejs/16.20.2/bin/node
  Yarn:
    version: 1.22.19
    path: ~/.asdf/installs/nodejs/16.20.2/bin/yarn
  npm:
    version: 8.19.4
    path: ~/.asdf/plugins/nodejs/shims/npm
  Watchman:
    version: 2023.05.08.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.12.0
    path: /Users/jeff/.asdf/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.0
      - iOS 17.0
      - macOS 14.0
      - tvOS 17.0
      - visionOS 1.0
      - watchOS 10.0
  Android SDK:
    API Levels:
      - "31"
      - "32"
      - "33"
    Build Tools:
      - 30.0.3
      - 31.0.0
      - 33.0.0
      - 33.0.1
    System Images:
      - android-31 | Google APIs ARM 64 v8a
      - android-33 | Google APIs ARM 64 v8a
    Android NDK: Not Found
IDEs:
  Android Studio: 2022.1 AI-221.6008.13.2211.9619390
  Xcode:
    version: 15.0/15A5229m
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 11.0.17
    path: /Users/jeff/.asdf/shims/javac
  Ruby:
    version: 2.7.6
    path: /Users/jeff/.asdf/shims/ruby
npmPackages:
  "@react-native-community/cli":
    installed: 9.3.3
    wanted: ^9.0.0
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.72.0
    wanted: 0.72.0
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: false
  newArchEnabled: false
iOS:
  hermesEnabled: false
  newArchEnabled: false

Steps to reproduce

DEBUG Build any RN iOS with a target of iOS >= 16.4 and see that it is not available for inspection per the documentation.

Snack, screenshot, or link to a repository

Any RN app will demonstrate this, so the base snack should show when configured with a DEBUG build.

https://snack.expo.dev/TRFqpEjAi

@github-actions github-actions bot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Sep 16, 2023
@jefflewis
Copy link
Author

I'll add a comment that this may also be the appropriate place to name the JSContext with a descriptive label (e.g. "React Native").

JSGlobalContextSetName`(jsContextRef, JSStringCreateWithUTF8CString("React Native"));

@github-actions github-actions bot removed the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Sep 16, 2023
@NickGerleman
Copy link
Contributor

@Saadnajmi didn't you add something for this?

@NickGerleman
Copy link
Contributor

Ah, found it. 8b1bf05

@jefflewis
Copy link
Author

One comment here though. The max target check would prevent an iOS device from being used for debugging, no?

@Saadnajmi
Copy link
Contributor

I also have an open change for naming, that hasn’t merged yet: #38942

@Saadnajmi
Copy link
Contributor

One comment here though. The max target check would prevent an iOS device from being used for debugging, no?

TARGET_OS_MAC includes iOS, surprisingly. Is that what you were concerned about?

@Saadnajmi
Copy link
Contributor

One comment here though. The max target check would prevent an iOS device from being used for debugging, no?

Wait sorry I think I had your comment confused. If you're referring to the __builtin_available check, I believe that before iOS 16.4, when there was no inspectable property, JScontexts' were always debuggable?

@jefflewis
Copy link
Author

Oh, you had it right, I was referring to TARGET_OS_MAC not including iOS. 😅

So I have no issues with the referenced implementations -- they appear to cover everything I was wanting to acheive!

@Saadnajmi
Copy link
Contributor

@jefflewis For the latter change, you need to access your JSExecutor to name your context. I didn't find a more convenient way like using your apps bundle name as the default.. let me know if you have suggestions there :D

@Saadnajmi
Copy link
Contributor

Heh.. speaking of that earlier change.. turns out I didn't properly support macOS... sooo _JSC_HAS_INSPECTABLE round 2!
#39549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants