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

Convert ReactPropConstantsTest to Kotlin #39005

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ public BoxedColorPropSetter(ReactProp prop, Method setter) {
*/
/*package*/ static Map<String, PropSetter> getNativePropSettersForShadowNodeClass(
Class<? extends ReactShadowNode> cls) {
if(cls == null) {
return new HashMap<>();
cortinico marked this conversation as resolved.
Show resolved Hide resolved
};

Copy link
Contributor Author

@thiagobrez thiagobrez Aug 14, 2023

Choose a reason for hiding this comment

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

Added null-check because cls was coming null when calling this function recursively with cls.getSuperClass() in lines 410-413, causing a NullPointerException

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change needed? Was one of your test failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cortinico Exactly, testNativePropsIncludeCorrectTypes was failing with a NullPointerException when instantiating the UIManagerModule:

val uiManagerModule = UIManagerModule(reactContext, viewManagers, 0)

I tracked it all the way down to ViewManagersPropertyCache.java, in getNativePropSettersForShadowNodeClass:

When this function is called recursively (in lines 410-413) with cls.getSuperclass(), the superclass might be null, causing cls.getInterfaces() (line 400) to crash.

for (Class iface : cls.getInterfaces()) {
  ...
}

Let me know if you believe there is a better way to solve this!

for (Class iface : cls.getInterfaces()) {
if (iface == ReactShadowNode.class) {
return EMPTY_PROPS_MAP;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.uimanager

import android.view.View
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.ReadableArray
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.common.MapBuilder
import com.facebook.react.uimanager.annotations.ReactProp
import com.facebook.react.uimanager.annotations.ReactPropGroup
import org.assertj.core.api.Assertions
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.RuntimeEnvironment

/** Verifies that prop constants are generated properly based on `ReactProp` annotation. */
@RunWith(RobolectricTestRunner::class)

class ReactPropConstantsTest {
private inner class ViewManagerUnderTest : ViewManager<View?, ReactShadowNode<*>?>() {
override fun getName(): String {
return "SomeView"
}

override fun createShadowNodeInstance(): ReactShadowNode<*>? {
Assertions.fail<Any>("This method should not be executed as a part of this test")
return null
}

override fun createViewInstance(reactContext: ThemedReactContext): View {
Assertions.fail<Any>("This method should not be executed as a part of this test")
return View(reactContext)
}

override fun getShadowNodeClass(): Class<out ReactShadowNode<*>> {
return ReactShadowNode::class.java
}

override fun updateExtraData(root: View, extraData: Any) {
Assertions.fail<Any>("This method should not be executed as a part of this test")
}

@ReactProp(name = "boolProp")
fun setBoolProp(v: View?, value: Boolean) {
}

@ReactProp(name = "intProp")
fun setIntProp(v: View?, value: Int) {
}

@ReactProp(name = "floatProp")
fun setFloatProp(v: View?, value: Float) {
}

@ReactProp(name = "doubleProp")
fun setDoubleProp(v: View?, value: Double) {
}

@ReactProp(name = "stringProp")
fun setStringProp(v: View?, value: String?) {
}

@ReactProp(name = "boxedBoolProp")
fun setBoxedBoolProp(v: View?, value: Boolean?) {
}

@ReactProp(name = "boxedIntProp")
fun setBoxedIntProp(v: View?, value: Int?) {
}

@ReactProp(name = "arrayProp")
fun setArrayProp(v: View?, value: ReadableArray?) {
}

@ReactProp(name = "mapProp")
fun setMapProp(v: View?, value: ReadableMap?) {
}

@ReactPropGroup(names = ["floatGroupPropFirst", "floatGroupPropSecond"])
fun setFloatGroupProp(v: View?, index: Int, value: Float) {
}

@ReactPropGroup(names = ["intGroupPropFirst", "intGroupPropSecond"])
fun setIntGroupProp(v: View?, index: Int, value: Int) {
}

@ReactPropGroup(names = ["boxedIntGroupPropFirst", "boxedIntGroupPropSecond"])
fun setBoxedIntGroupProp(v: View?, index: Int, value: Int?) {
}

@ReactProp(name = "customIntProp", customType = "date")
fun customIntProp(v: View?, value: Int) {
}

@ReactPropGroup(
names = ["customBoxedIntGroupPropFirst", "customBoxedIntGroupPropSecond"],
customType = "color"
)
fun customIntGroupProp(v: View?, index: Int, value: Int?) {
}
}

@Test
fun testNativePropsIncludeCorrectTypes() {
val viewManagers = listOf<ViewManager<*, *>>(ViewManagerUnderTest())
val reactContext = ReactApplicationContext(RuntimeEnvironment.getApplication())
val uiManagerModule = UIManagerModule(reactContext, viewManagers, 0)
val constants: Map<*, *>? = valueAtPath(uiManagerModule.constants, "SomeView", "NativeProps")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
val constants: Map<*, *>? = valueAtPath(uiManagerModule.constants, "SomeView", "NativeProps")
val constants: Map<*, *> = valueAtPath(uiManagerModule.constants, "SomeView", "NativeProps")


Assertions.assertThat(constants)
.isEqualTo(
MapBuilder.builder<String, String>()
.put("boolProp", "boolean")
.put("intProp", "number")
.put("doubleProp", "number")
.put("floatProp", "number")
.put("stringProp", "String")
.put("boxedBoolProp", "boolean")
.put("boxedIntProp", "number")
.put("arrayProp", "Array")
.put("mapProp", "Map")
.put("floatGroupPropFirst", "number")
.put("floatGroupPropSecond", "number")
.put("intGroupPropFirst", "number")
.put("intGroupPropSecond", "number")
.put("boxedIntGroupPropFirst", "number")
.put("boxedIntGroupPropSecond", "number")
.put("customIntProp", "date")
.put("customBoxedIntGroupPropFirst", "color")
.put("customBoxedIntGroupPropSecond", "color")
.build()
)
}

companion object {
private fun valueAtPath(nestedMap: Map<String, *>?, vararg keyPath: String): Map<*, *>? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private fun valueAtPath(nestedMap: Map<String, *>?, vararg keyPath: String): Map<*, *>? {
private fun valueAtPath(nestedMap: Map<String, *>, vararg keyPath: String): Map<*, *> {

Assertions.assertThat(keyPath).isNotEmpty
var value = nestedMap
for (key in keyPath) {
Assertions.assertThat(value).isInstanceOf(Map::class.java)
Assertions.assertThat(value).containsKey(key)
value = (value?.get(key) as? Map<String, *> ?: error("Value not found for key: $key"))
}
return value
}
}
}