[react] Component and Builder APIs need cleanup

[fluidsonic]

When I've started working with kotlin-react I've had a lot of confusion because the API is not consistent nor easy to understand.

To name a few:

• Some things work with class-based components but not with functional components and vice versa.
• The provided types are inconsistently named and their hierarchy confusing.
• The relations between and intent of the various types are confusing.
• Some functionality was quite difficult to implement, e.g. using styled as HOC for functional components.

Status quo

I'll go through a collection of inconsistencies that I've noticed.

There are two functions to create functional components that are almost identical:

kotlin-wrappers/kotlin-react/src/main/kotlin/react/RBuilder.kt

Lines 176 to 189 in 10805ef

	fun <P : RProps> functionalComponent(
	displayName: String? = null,
	func: RBuilder.(props: P) -> Unit
	): FunctionalComponent<P> {
	val fc = { props: P ->
	buildElements {
	func(props)
	}
	}
	if (displayName != null) {
	fc.asDynamic().displayName = displayName
	}
	return fc
	}

kotlin-wrappers/kotlin-react/src/main/kotlin/react/ReactComponent.kt

Lines 84 to 91 in 10805ef

	fun <P : RProps> rFunction(
	displayName: String,
	render: RBuilder.(P) -> Unit
	): RClass<P> {
	val fn = { props: P -> buildElements { render(props) } }
	return fn.unsafeCast<RClass<P>>()
	.also { it.displayName = displayName }
	}

A crucial difference is that one returns RClass<P> while the other returns FunctionalComponent<P>.

• From React's point of view, both are component definitions.
• From Kotlin's point of view, RClass<P> and FunctionalComponent<P> have no common interface and thus cannot be used interchangeably where a component definition is expected.
• In rFunction above the FunctionalComponent is even cast to RClass<P>.
• RClass has the omnipresent R prefix while FunctionalComponent does not.

That makes it difficult to use API that is made for either one or the other but could easily be used for both, for example:

• HOC operates on RClass.
• lazy/rLazy() operate on RClass.
• styled() operates on RClass.
• memo() operates on FunctionalComponent.

Then there's also RBuilder.child() which is inconsistent across overloads:

• One operates on Child, i.e. not a component but an actual element. The very generic name Child is confusing because components are also added as children, but aren't a Child.
• One operates on Any and is type-unsafe. handler is a required parameter. props is a required parameter.
• One operates on KClass<out Component>. handler is a required parameter. No props parameter.
• One operates on reified Component. handler is a required parameter. No props parameter.
• One operates on FunctionalComponent. handler is an optional parameter. props is an optional parameter. It's an extension function.
• None operates on RClass explicitly.

It's not much better with SomeType.invoke() within RBuilder.

• One operates on RClass. handler is a required parameter.
• None operates on FunctionalComponent.
• handler shouldn't be needed if P = RProps.

The distinction between Component and RComponent is also not clear.

The bottom line

• It's very difficult for library users to make sense of how RClass, (R)Component and FunctionalComponent are related.
• It's very difficult to use HOC consistently independently of how you've defined a component.
• It's very confusing that RBuilder's child() and invoke() are defined so inconsistently across different types of component definitions.

Ideas for improvement

I suggest starting with cleaning up the type hierarchy. Something along these lines:

interface RComponentDefinition<P: RProps> {
    var defaultProps: P?
    var displayName: String?
}

interface RComponentClass<P: RProps, S: RState>: RComponentDefinition<P> {
    var contextType: RContext<Any>?
    var getDerivedStateFromError: ((Throwable) -> S?)?
    var getDerivedStateFromProps: ((P, S) -> S?)?
}

interface RFunctionalComponent<P: RProps>: RComponentDefinition<P>, (P) -> dynamic

• All HOCs an other functions that take component definitions would operate on RComponentDefinition.
• Additionally they may add overloads for KClass<out RComponentClass> as we cannot have KClass<out RComponentClass> implement RComponentClass.
• It would be great to have RState and RContext across the entire hierarchy. But that would make the API way more complicated because you'd have to specify 3 generic parameters everywhere. This would benefit hugely from KT-1215 and KT-17061.
• P of RComponentDefinition probably shouldn't be in-projected because it also uses it in an out position (defaultProps).

RBuilder.child() could be improved like this:

// all within RBuilder scope

// props needed, handler needed
fun <P: RProps> child(
   definition: RComponentDefinition<P>,
   handler: RHandler<P>
): ReactElement

// props given, no handler needed
fun <P: RProps> child(
   definition: RComponentDefinition<P>,
   props: P,
   handler: RHandler<P> = {}
): ReactElement

// no props needed, no handler needed
fun child(
   definition: RComponentDefinition<RProps>
): ReactElement

// props needed, handler needed
fun <P: RProps> child(
   definition: KClass<out Component<P, *>>,
   handler: RHandler<P>
): ReactElement

// props given, no handler needed
fun <P: RProps> child(
   definition: KClass<out Component<P, *>>,
   props: P,
   handler: RHandler<P> = {}
): ReactElement

// no props needed, no handler needed
fun child(
   definition: KClass<out Component<RProps, *>>
): ReactElement

• The above is consistent between class-based and functional components.
• It allows providing props as parameter and through handler.
• It allows omitting the handler if none is needed.

RBuilder.invoke() could be improved like this:

// all within RBuilder scope

// props needed, handler needed
operator fun <P: RProps> RComponentDefinition<P>.invoke(
   handler: RHandler<P>
): ReactElement

// props given, no handler needed
operator fun <P: RProps> RComponentDefinition<P>.invoke(
   props: P,
   handler: RHandler<P> = {}
): ReactElement

// no props needed, no handler needed
operator fun RComponentDefinition<RProps>.invoke(): ReactElement

// props needed, handler needed
operator fun <P: RProps> KClass<out Component<P, *>>.invoke(
   handler: RHandler<P>
): ReactElement

// props given, no handler needed
operator fun <P: RProps> KClass<out Component<P, *>>.invoke(
   props: P,
   handler: RHandler<P> = {}
): ReactElement

// no props needed, no handler needed
operator fun KClass<out Component<RProps, *>>.invoke(): ReactElement

• The above is consistent between class-based and functional components.
• It allows providing props as parameter and through handler.
• It allows omitting the handler if none is needed.

There would also have to be a replacement for KClass<out Component<P, *>>.rClass to get a RComponentDefinition<P> instead.

Last but not least, let's remove either rFunction or functionalComponent and have the remaining one return RFunctionalComponent<P>.

[andylamax]

I know the team is busy and all right now but, I just wanted to say, I know this is a breaking change but it is a necessary one. Please please lets solve this once and for all

[fluidsonic]

@andylamax that's why I've started my own wrapper:
https://github.com/fluidsonic/fluid-react

[Skolotsky]

Yes, we haven't enough time now, and i know that it's necessary, but you still can make PRs to help us.

[sgrishchenko]

@fluidsonic First of all, thank you for your review. We performed a lot of work to improve our API after your notes and I will try to analyze results.

Some things work with class-based components but not with functional components and vice versa.

We introduced ComponentType to solve it

The provided types are inconsistently named and their hierarchy confusing.

We tried to align with Typescript typings for React, we reduced R-naming and added some missing concepts

The relations between and intent of the various types are confusing.

We tried to simplify API and implement all common functionality using ComponentType interface

Some functionality was quite difficult to implement, e.g. using styled as HOC for functional components.

Right now we are working on CSS improvement and thinking about reincarnation of CSS-in-JS solution in wrappers. But now there are much less difficulties with types system because again we have ComponentType

After all I propose to close this issue and open a new one if there are additional suggestion for improvements.

cc @Leonya @turansky