SimonDanisch / Bonito.jl

Serving JS to the browser
MIT License
212 stars 31 forks source link

notify also on falsy values #236

Closed piever closed 3 months ago

piever commented 3 months ago

On the main branch, the if clause before updating the value on JS will prevent the update when value is "falsy" (0, "", null, etc).

Simplest way to test is to run

using Bonito
obs = Observable(1)
app = App() do session
    evaljs(session, js"""
    $obs.notify(0, true);
    console.log($obs.value)
    """)
end

on main, the console log shows 1 (no update occurred) and on this PR it shows 0.

I wonder whether the idea was to allow obs.notify(), intending to not update the observable and only trigger callbacks but

SimonDanisch commented 3 months ago

Oh wow, good old JS anti-patterns... I feel like there was a reason for this, but can't find any notify() in the code base...

piever commented 3 months ago

(Fixed the new test.)

I feel like there was a reason for this, but can't find any notify() in the code base...

Me nether... But to be honest I'd rather avoid it (and keep the PR as is). AFAIU, javascript would not distinguish between notify() and notify(x.a) for example where the field a is not present (both give value = undefined), which seems error prone... One can always do obs.notify(obs.value) if needed.