Skip to content

Commit 94e8ccf

Browse files
usikderfacebook-github-bot
authored andcommitted
BREAKING: rm YogaNode parameter from YogaLogger#log
Summary: In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed. Attempt #1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger. Attempt #2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile. At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv). Reviewed By: amir-shalem Differential Revision: D17470379 fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2
1 parent e028ac7 commit 94e8ccf

File tree

4 files changed

+9
-17
lines changed

4 files changed

+9
-17
lines changed

ReactAndroid/src/main/java/com/facebook/yoga/YogaLogger.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@
1515
@DoNotStrip
1616
public interface YogaLogger {
1717
@DoNotStrip
18-
void log(YogaNode node, YogaLogLevel level, String message);
18+
void log(YogaLogLevel level, String message);
1919
}

ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJNI.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,10 @@ static int YGJNILogFunc(
311311
auto jloggerPtr =
312312
static_cast<global_ref<JYogaLogger>*>(YGConfigGetContext(config));
313313
if (jloggerPtr != nullptr) {
314-
if (auto obj = YGNodeJobject(node, layoutContext)) {
315-
(*jloggerPtr)
316-
->log(
317-
obj,
318-
JYogaLogLevel::fromInt(level),
319-
Environment::current()->NewStringUTF(buffer.data()));
320-
}
314+
(*jloggerPtr)
315+
->log(
316+
JYogaLogLevel::fromInt(level),
317+
Environment::current()->NewStringUTF(buffer.data()));
321318
}
322319

323320
return result;

ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,10 @@ facebook::jni::local_ref<JYogaLogLevel> JYogaLogLevel::fromInt(jint logLevel) {
3333
}
3434

3535
void JYogaLogger::log(
36-
facebook::jni::alias_ref<JYogaNode> node,
3736
facebook::jni::alias_ref<JYogaLogLevel> logLevel,
3837
jstring message) {
3938
static auto javaMethod =
40-
javaClassLocal()
41-
->getMethod<void(
42-
alias_ref<JYogaNode>, alias_ref<JYogaLogLevel>, jstring)>("log");
43-
javaMethod(self(), node, logLevel, message);
39+
javaClassLocal()->getMethod<void(alias_ref<JYogaLogLevel>, jstring)>(
40+
"log");
41+
javaMethod(self(), logLevel, message);
4442
}

ReactAndroid/src/main/jni/first-party/yogajni/jni/YGJTypes.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@ struct JYogaLogLevel : public facebook::jni::JavaClass<JYogaLogLevel> {
2828
struct JYogaLogger : public facebook::jni::JavaClass<JYogaLogger> {
2929
static constexpr auto kJavaDescriptor = "Lcom/facebook/yoga/YogaLogger;";
3030

31-
void log(
32-
facebook::jni::alias_ref<JYogaNode>,
33-
facebook::jni::alias_ref<JYogaLogLevel>,
34-
jstring);
31+
void log(facebook::jni::alias_ref<JYogaLogLevel>, jstring);
3532
};
3633

3734
class PtrJNodeMap {

0 commit comments

Comments
 (0)