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.

Sign up for GitHub

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 Symbol not found: (_JSGlobalContextSetInspectable) #44185

Closed
wants to merge 1 commit into from
Closed

Fix Symbol not found: (_JSGlobalContextSetInspectable) #44185

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/react-native/ReactCommon/jsc/JSCRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,13 @@ JSCRuntime::JSCRuntime(JSGlobalContextRef ctx)
{
#ifndef NDEBUG
#ifdef _JSC_HAS_INSPECTABLE
#if (__OSX_AVAILABLE_STARTING(MAC_NA, IPHONE_16_4))
Comment on lines 379 to +380
Copy link
Contributor

@Saadnajmi Saadnajmi Apr 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@cipolleschi Original author of _JSC_HAS_INSPECTABLE, should we move this check to the macros' definition (AKA, line 303)? I can do so in my already open PR where I refactor some of it #39549

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Saadnajmi, yes, we can. I just saw we had a check on the iOS version at line 303, and I think that that's what triggered the issue... The version is set to 160400, which is... 160.4!!!
I think it should be 16400 if we want to specify 16.4.

- #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 160400
+ #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 16400

for example, the check for iOS 17, is 17000.

Copy link
Contributor

Choose a reason for hiding this comment

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

Damn. I remember double checking that. Ok, I guess we don't need the double check then, and I'll fix the versions.

Copy link
Contributor

@Saadnajmi Saadnajmi Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Also closing this thread so we can keep discussion in #39549 (comment) , but as noted there, I think the existing macro is correct.
EDIT: I can't actually resolve this thread.. so.. just go comment on the other one :D

if (__builtin_available(macOS 13.3, iOS 16.4, tvOS 16.4, *)) {
JSGlobalContextSetInspectable(ctx_, true);
}
#endif
#endif
#endif
}

JSCRuntime::~JSCRuntime() {
Expand Down
Loading