Trying out signals in Flow with Vaadin 24.8

Is it save to assume, that Signal::map, which ends up in
a ComputedSignal, is the proper way to “zip” signals? Empirical
evidence seems fine.

E.g.

val tick = ValueSignal(Instant.now().toEpochMilli())
val zoneName: ValueSignal<String> = ValueSignal(ZoneId.systemDefault().toString())

val now: Signal<LocalTime> = tick.map { // XXX
    Instant
        .ofEpochMilli(it)
        .atZone(
            ZoneId.of(
                zoneName.value()        // XXX
            )
        )
        .toLocalTime()
}
My full example
class ClockSegment(number: Signal<Int>) : Span() {
    init {
        ComponentEffect.bind(this, number, { c, v -> c.setText("%02d".format(v)) })
        style.set("font-family", "monospace")
    }
}

class DayOrNight(hour: Signal<Int>) : Span() {

    val day = hour.map { it in 6..18 }
    val night = day.map { !it }

    init {
        val dayDisplay = VaadinIcon.SUN_O.create()
        val nightDisplay = VaadinIcon.MOON_O.create()
        add(dayDisplay, nightDisplay)
        ComponentEffect.bind(dayDisplay, day, Component::setVisible)
        ComponentEffect.bind(nightDisplay, night, Component::setVisible)
    }
}

class Clock(value: Signal<LocalTime>) : Div() {

    val hour = value.map(LocalTime::getHour)
    val minute = value.map(LocalTime::getMinute)
    val second = value.map(LocalTime::getSecond)

    init {
        add(
            DayOrNight(hour),
            ClockSegment(hour), Span(":"),
            ClockSegment(minute), Span(":"),
            ClockSegment(second)
        )
    }
}

@Route(value = "", layout = MainLayout::class)
@PageTitle("Main")
class MainView() : Div() {

    init {
        val tick = ValueSignal(Instant.now().toEpochMilli())
        val zoneName: ValueSignal<String> = ValueSignal(ZoneId.systemDefault().toString())
        val zones = ComboBox<String>().apply {
            setItems(ZoneId.getAvailableZoneIds().sorted())
            setValue(zoneName.value())
            // Fails because already in TX ComponentEffect.bind(this, zoneName, ComboBox<String>::setValue )
            addValueChangeListener { zoneName.value(it.value) }
        }
        val now: Signal<LocalTime> = tick.map {
            Instant
                .ofEpochMilli(it)
                .atZone(
                    ZoneId.of(
                        zoneName.value()
                    )
                )
                .toLocalTime()
        }
        val clock = Clock(now)
        add(clock, zones)
        Executors.newScheduledThreadPool(1).scheduleAtFixedRate({
            clock.ui.map {
                it.access {
                    tick.value(Instant.now().toEpochMilli())
                }
            }
        }, 1, 1, TimeUnit.SECONDS)
    }

}

That usage pattern is safe but I recommend against it for clarity reasons. I believe it’s a good practice to keep .value() calls explicit in any non-trivial case to make it clear which parts of the calculation are dynamic. I would thus recommend using the Signal.computed factory method for creating that computed signal with two explicit .value() calls.

1 Like

Yeah, that’s why I will add a zip method for myself here. I would also
like to have the clarity, but I want it on the “input” side. Burying
the .value() calls inside code (I understand, why it is how it is),
does not communicate intend very well (e.g. like using for/if instead
of filter/predicate). For that reason I don’t like
Signal::computed; I’ll rather use ComputedSignal directly, but that
being inside an impl namespace makes me think it will vanish some day.

The intent behind map is mostly to use it as a shorthand for the really simple cases like stringSignal.map(String::isEmpty) or such while using Signal.computed for everything else. Getting one of the inputs as a callback parameter (or it as in your Kotlin example) and ther other as a .value() call is IMHO the “worst” way since you then have two very similar things expressed in different ways.

Java doesn’t give any way of doing a generic zip function with support for variable number of inputs so it would instead be necessary to have one overload for each number, (e.g. public static <T, U, V> Signal<T> zip(Signal<U> i1, Signal<V> i2, BiFunction<U, V, T> zipper)). We could consider providing these up to some reasonable number of arguments similar to the Map.of overloads in Java.

At the same time, this pattern is sub-optimal in any case where some of the signals would be used conditionally since the zip signal would still end up depending on all of them whereas an approach with explicit .value() calls would depend only on the ones that were actually used during the last invocation.

Signal.computed(() -> {
  return booleanSignal.value() ? stringSignal1.value() : stringSignal2.value();
});

There are also cases with a dynamic number of signals that requires a pattern with explicit .value() calls like the lines.map(list -> list.stream().mapToInt(line -> line.value().quantity()).sum()) case from the Orders example that I shared. This leads me to thinking that it might be better to help developers to get familar with the .value() structure also in other cases so that they realize it’s there when it’s really needed. I also think there’s some merit to encouraging the same structure for effect and computed

(On the other hand, I also conjured an iteration helper for ListSignal to simplify that use case into lines.mapChildValues(values -> values.mapToInt(OrderLine::quantity).sum()).)

Yet another concern on my mind is variable naming since a zip function forces you to introduce additional variables.

Signal.zip(foo, bar, (fooValue, barValue) -> fooValue + barValue));

Compare this to direct .value() that doesn’t require any additional variable names

Signal.computed(() -> foo.value() + bar.value());

One variant that might help provide clarity for non-trivial calculations could be to do the actual calculation in a separate pure function and only do the inputs and outputs in the actual computed signal definition.

Signal.computed(() -> doSomething(foo.value(), bar.value());

Though with this pattern, zip also becomes quite practical

Signal.zip(foo, bar, SomeClass::doSomething);

Not sure I get the distinction there since those are equivalent with regards to how inputs are treated? The main reason for designing the API in that way is to improve discoverability by using Signal as an entry point for most operations rather than expecting the developer to remember many different names to start from.

1 Like

You are right about my use of map – I was fine with it here, because the tick
is the dominating driver of the change and there will maybe rare changes
of the zone.

I am also totally fine, that zip or other helpers will not be added to
the core, because of the mentioned arities, naming, and when to stop (see
Java’s Gatherers). Also Vaadin is primarily OO, so why add FP stuff.

The example with the booleanSignal is also something I’d not use zip
for (for the mentioned short-circuiting problem). But once the problem
arises, I’d add a helper in my project (taking a Bool and two T
signals and naming it conditonal or ifThenElse).

About “always using computed”:

This is not about saving on typing letters for me. This is only for
communicating intent. E.g.:

Signal.computed(() -> foo.value() + bar.value());

I have to read this code fully to understand it. There is nothing,
that helps me to understand it at a glance. I have to brain-scan for
.value() to find the signal usage and then parse the rest on how
everything interacts (it’s trivial for this short example, but my now
example already becomes more involved).

Signal.zip(foo, bar, (fooValue, barValue) -> fooValue + barValue));

This looks verbose, because Java does not have an operator for it. What
if I could write:

Signal.zip(foo, bar, Integer::add)

And with:

Signal.computed(() -> doSomething(foo.value(), bar.value());
// becomes
Signal.zip(foo, bar, doSomething)

And finally about Signal::complete – I missed, that it’s static.
Makes total sense to use it over ComputedSignal. Thanks for pointing
it out.

1 Like

Currently only java.time is optionally supported via Jackson in
VaadinService#createDefaultObjectMapper to send to the client.

This is also true when Spring is involved. There is no override in the
spring service, that uses the ObjectMapper Spring provides.

Is this intentional, because this will never (properly) work with
anything but JSON base-types? Or something to come?

Or is there a way to tell a signal, it will/must not be sent to the
client?

I have encountered the same challenge. Note though that it’s not only about being sent to the client but it’s also relevant for clustering. Finally, the deep clone is an excellent protection against accidental modification in a way that wouldn’t be detected.

At the same time, I’ve played with the idea of an InstanceSignal<T> that would explicitly not take the value through Jackson.