Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Commit

Permalink
fix: problems observed while refactoring Nimbus for Compose (#54)
Browse files Browse the repository at this point in the history
* fixes problems observed while adapting Nimbus Compose to the new Core update strategy

* self-review
  • Loading branch information
Tiagoperes authored Sep 29, 2022
1 parent 08c71d3 commit e2a4ccc
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 29 deletions.
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ kotlin.native.binary.memoryModel=experimental

#POM CONFIGURATION
POM_NAME=nimbus
VERSION_NAME=1.0.0-alpha5
VERSION_NAME=1.0.0-alpha
POM_GROUP=br.com.zup.nimbus
POM_DESCRIPTION=A framework to help implement Server-Driven UI in your apps natively.
POM_INCEPTION_YEAR=2022
Expand Down
2 changes: 1 addition & 1 deletion src/commonMain/kotlin/com/zup/nimbus/core/Nimbus.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import com.zup.nimbus.core.ui.coreUILibrary
/**
* The root scope of a nimbus application. Contains important objects like the logger and the httpClient.
*/
class Nimbus(config: ServerDrivenConfig): CommonScope(
open class Nimbus(config: ServerDrivenConfig): CommonScope(
parent = null,
states = (config.states ?: emptyList()) + ServerDrivenState("global", null),
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.zup.nimbus.core.deserialization

import com.zup.nimbus.core.log.Logger
import com.zup.nimbus.core.tree.ServerDrivenEvent
import com.zup.nimbus.core.tree.ServerDrivenNode

class ComponentDeserializer(val logger: Logger, val node: ServerDrivenNode) {
Expand Down Expand Up @@ -58,8 +59,8 @@ class ComponentDeserializer(val logger: Logger, val node: ServerDrivenNode) {
return deserializer.asMap(key)
}

fun asAction(key: String): (Any?) -> Unit {
return deserializer.asAction(key)
fun asEvent(key: String): ServerDrivenEvent {
return deserializer.asEvent(key)
}

fun asStringOrNull(key: String): String? {
Expand Down Expand Up @@ -90,7 +91,7 @@ class ComponentDeserializer(val logger: Logger, val node: ServerDrivenNode) {
return deserializer.asMapOrNull(key)
}

fun asActionOrNull(key: String): ((Any?) -> Unit)? {
return deserializer.asActionOrNull(key)
fun asEventOrNull(key: String): ServerDrivenEvent? {
return deserializer.asEventOrNull(key)
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.zup.nimbus.core.deserialization

import com.zup.nimbus.core.tree.ServerDrivenEvent
import com.zup.nimbus.core.tree.dynamic.DynamicEvent

class MapDeserializer {
private var errors = ArrayList<String>()
private var current: Map<String, Any?>? = null
Expand Down Expand Up @@ -72,13 +75,12 @@ class MapDeserializer {
return emptyMap()
}

private fun getAction(key: String, nullable: Boolean): ((Any?) -> Unit)? {
private fun getEvent(key: String, nullable: Boolean): ServerDrivenEvent? {
val value = current?.get(key)
if (nullable && value == null) return null
@Suppress("UNCHECKED_CAST")
if (value is Function1<*, *>) return value as (Any?) -> Unit
addTypeError("an array of Actions", key, value)
return { _ -> }
if (value is ServerDrivenEvent) return value
addTypeError("an event, i.e. an array of actions.", key, value)
return DynamicEvent("Unknown")
}

fun start(map: Map<String, Any?>?) {
Expand Down Expand Up @@ -134,8 +136,8 @@ class MapDeserializer {
return getMap(key, false)!!
}

fun asAction(key: String): (Any?) -> Unit {
return getAction(key, false)!!
fun asEvent(key: String): ServerDrivenEvent {
return getEvent(key, false)!!
}

fun asStringOrNull(key: String): String? {
Expand Down Expand Up @@ -166,7 +168,7 @@ class MapDeserializer {
return getMap(key, true)
}

fun asActionOrNull(key: String): ((Any?) -> Unit)? {
return getAction(key, true)
fun asEventOrNull(key: String): ServerDrivenEvent? {
return getEvent(key, true)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ import io.ktor.http.Headers
import io.ktor.http.HttpMethod
import kotlin.collections.set

class DefaultHttpClient internal constructor (
engine: HttpClientEngine? = null,
): com.zup.nimbus.core.network.HttpClient {
class DefaultHttpClient (engine: HttpClientEngine? = null): com.zup.nimbus.core.network.HttpClient {
constructor() : this(null)

private val client = if (engine == null) HttpClient() else HttpClient(engine)
Expand Down
2 changes: 1 addition & 1 deletion src/commonMain/kotlin/com/zup/nimbus/core/scope/Scope.kt
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ interface Scope {
/**
* Returns the closest Scope with the specified type.
*/
internal inline fun <reified T: Scope> Scope.closestScopeWithType(): T? {
inline fun <reified T: Scope> Scope.closestScopeWithType(): T? {
var current: Scope? = this
while (current != null && current !is T) current = current.parent
return current?.let { current as T }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@ package com.zup.nimbus.core.tree.dynamic.node
import com.zup.nimbus.core.ServerDrivenState
import com.zup.nimbus.core.utils.valueOfKey

// fixme: normally, in UI frameworks, if-else blocks completely remove the other branch from the tree and rebuilds it
// fixme(1): normally, in UI frameworks, if-else blocks completely remove the other branch from the tree and rebuilds it
// when the condition changes. This is not being done here. On the positive side, states will never be lost when
// switching from true to false. On the other hand, we won't free up the memory for the if-else branch not currently
// rendered until the associated RootNode is unmounted. If we decide this to be a feature and not a bug, remove this
// commentary.
//
// fixme(2): we shouldn't try to run the components of Else if the condition is true; or the components of Then when
// the condition is false. Example: we may use an operation that expects something not to be null and this can only be
// guaranteed by the if's condition. By initializing both Then and Else no matter the condition value, we'll end up
// processing way more than we need and risk running operations that expect values different than the ones we have.
// PS: fixing the first issue automatically fixes this.
/**
* IfNode is a polymorphic DynamicNode that chooses only one of its original subtree (json) to render at a time. The
* chosen subtree depends on the value of the property "condition". When "condition" is true and "Then" is a child of
Expand Down
13 changes: 4 additions & 9 deletions src/commonMain/kotlin/com/zup/nimbus/core/ui/UILibrary.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@ open class UILibrary(
private val actionObservers = mutableListOf<ActionHandler>()
private val operations = mutableMapOf<String, OperationHandler>()

fun addAction(name: String, handler: ActionHandler): UILibrary {
open fun addAction(name: String, handler: ActionHandler): UILibrary {
actions[name] = handler
return this
}

fun addActionInitializer(name: String, handler: ActionInitializationHandler): UILibrary {
open fun addActionInitializer(name: String, handler: ActionInitializationHandler): UILibrary {
actionInitializers[name] = handler
return this
}

fun addActionObserver(observer: ActionHandler): UILibrary {
open fun addActionObserver(observer: ActionHandler): UILibrary {
actionObservers.add(observer)
return this
}

fun addOperation(name: String, handler: OperationHandler): UILibrary {
open fun addOperation(name: String, handler: OperationHandler): UILibrary {
operations[name] = handler
return this
}
Expand All @@ -65,11 +65,6 @@ open class UILibrary(
return operations[name]
}

/**
* Merges the given UILibrary into this UILibrary. This alters the current UILibrary.
*
* Attention: remember to override this method when extending this class.
*/
open fun merge(other: UILibrary): UILibrary {
actions.putAll(other.actions)
actionInitializers.putAll(other.actionInitializers)
Expand Down

0 comments on commit e2a4ccc

Please sign in to comment.