Closed dead-claudia closed 2 weeks ago
Update: I have an alternative that might also be worth considering. We could instead just detect promises and, if they resolve to a truthy value or undefined
, redraw then rather than just always checking and making the decision synchronously. This provides a couple perks:
oncreate
, it provides an alternative to #2217 in that we could just drop the background
option altogether there.@dead-claudia we've had this conversation numerous times over the past 6 years, and I struggle to find a way to re-articulate the arguments in a meaningful way because they are perpetually ignored and then forgotten.
There is a logical contradiction in the proposal: we are proposing on the one hand a net value in removing a tiny amount of inexpensive code which implements conventionally expected behaviour; but then introducing more code to introduce new semantics ex-nihilo.
As regards the specifics of the value of return false
: I know you're not a fan of this behaviour, but it's the way the platform works. Rather than deciding whether or not to reimplement spec behaviour in-library, we can wash our hands of the dilemma by simply having the humility not to enforce an opinion either way:
// Current
var result
if (typeof handler === "function") result = handler.call(ev.currentTarget, ev)
else if (typeof handler.handleEvent === "function") handler.handleEvent(ev)
if (this._ && ev.redraw !== false) (0, this._)()
if (result === false) {
ev.preventDefault()
ev.stopPropagation()
}
// Better
var result
if (typeof handler === "function") result = handler.call(ev.currentTarget, ev)
else if (typeof handler.handleEvent === "function") result = handler.handleEvent(ev)
if (this._ && ev.redraw !== false) (0, this._)()
return result
If fulfilling native API parity required complex handling to account for contradictions with Mithril API overrides we deemed more valuable, we would be making a bold assertion! But as it is the handling is trivial — in this case, over-complicated by our own misapprehensions — and what we're currently seeing is the tentative development of such home-grown contradictions in order to justify that second-guessing of platform expectations.
The debate about special Promise interpretation is also old. Historically, v1's first iteration had the strong opinion that Mithril ought to extend the Promise implementation to try to determine when a Promise chain had eventually settled, and only redraw at such point: As I argued at length at the time, Promise chains can fork, and can do so asynchronously — meaning it is not determinable in the space of one tick whether or how such chains would ever fully settle, since they can be extended at any given point as they are still referenceable in application code — and it is in any case foolhardy to try to second-guess the semantics of any given async transaction as regards when the author of such code might expect the 'one true redraw' to occur.
Taking the example from the original post:
// As copied from the original post
async onclick() {
await doStuff()
return false // Doesn't work like it looks like it does!
}
An author running into this confusion will need to understand that the native APIs are no longer intercepting a false return but a promise with an eventual settlement of false; their argument is then between themselves and the platform.
But I think the confusion is less with the imagined author than on our side: m.request
already handles auto-redraw on completion. Let's take some slightly more fleshed out variations:
// Native return semantics trumps imperative async ergonomics
onclick() {
arbitraryFutureWithSideEffects().then(() => m.redraw())
return false
}
onclick() {
m.request(/**/).then(data => {
Object.assign(closuredState, {data})
})
return false
}
// Imperative async sequence trumps native return ergonomics
async onclick(e) {
e.preventDefault()
e.stopImmediatePropagation()
closuredState.loading = true
await arbitraryFutureWithSideEffects()
closuredState.loading = false
m.redraw()
}
async onclick(e) {
e.preventDefault()
e.stopImmediatePropagation()
closuredState.loading = true
Object.assign(closuredState, {data : await m.request(/**/)})
closuredState.loading = false
}
There's a bunch of things going on here, and different considerations for each.
arbitraryFutureWithSideEffects
, data persistence is happening as a side-effect but we're yielding outside of the Mithril idiom, so we take responsibility to redraw.m.request
, we're handling everything in-situ, and taking care of data persistence. m.request
redraws on completion anyway, so we don't need to invoke it.m.request
does the same, and variously take account of such.These are relatively simple scenarios, but they show that the Mithril user needs to have some grasp of how low-level native DOM & Mithril APIs work for event & redraw semantics so resolve with precision.
My contention is that our historic attempts to do away with DOM lvl 1 semantics and special case async side-effects are the result of internal confusion projected upon a conjectured naive user, but the paternalistic instinct is misguided:
The mindset behind these proposals assumes the user is too confused to understand what they are doing with predictable low-level APIs, and seeks to create special cases to ease that confusion: but the solutions complicate behaviour in ways whose conjectured 'simplicity' can't account for mundane variations in real world complexity — this makes things even more confusing for users who would credibly encounter such complexity but can no longer rely on the predictability of low-level APIs to account for such.
This is not to say that real Mithril users won't be confused by the essentials of event cancellation mechanics & async logic — everybody is at some point & to some degree and we should fully expect people bumping into thwarted expectations and asking for help — but the solution to such confusion is not to bake complexity into the library and litter the code and documentation with comments about 'intuition' and 'what you might expect' but to give simple & reliable controls whose behaviour is easily described, logically predictable, and flexible enough to build solutions depending on a variety of emergent use cases.
@dead-claudia What benefit would m.capture(event)
give over calling event.preventDefault()
and event.stopPropagation()
manually? Not bashing the idea, just curious.
From @barneycarroll's comment (emphasis added):
removing a tiny amount of inexpensive code which implements conventionally expected behaviour
As regards the specifics of the value of
return false
: […] it's the way the platform works.
My understanding is that returning false
from an event listener is a jQuery thing.
I created a Flems playground that shows that outside jQuery, returning false
from an event handler:
$el.addEventListener('click', () => {})
$el.addEventListener('click', { handleEvent() {} })
<button onclick="handler()">
(no return
)<button onclick="return handler()">
(notice the return
)$el.onclick = () => {}
Mithril uses addEventListener
, so Barney's suggestion to return result
from EventDict.prototype.handleEvent
would do nothing (would not stop event propagation and would not prevent the default action). Or maybe I'm amiss?
@mtsknn Consider it in light of https://github.com/MithrilJS/mithril.js/pull/2862 (I had that idea well before that PR, obviously).
Event listeners must synchronously prevent their actions. When using an async
function, the return value is not false
but a language-generated promise. What is the easiest way to ensure the event is captured and neither propagates further up nor performs the associated action (like adding a character), when you have no direct control over the return value? I say m.capture
is more or less the helper you'd write anyways after a point, initially as a higher order function until you found yourself wanting to conditionally disable it more than once or twice (which is relatively common for keyboard events outside form controls).
Consider it a compromise of sorts given the problem constraints.
Edit: fixed function name
Mithril uses
addEventListener
, so Barney's suggestion toreturn result
fromEventDict.prototype.handleEvent
would do nothing (would not stop event propagation and would not prevent the default action). Or maybe I'm amiss?
Barney's referring to the elem.onevent
setters and related HTML attributes, of which Mithril's event listener design is based on. Mithril internally uses addEventListener
, but aside from custom element interop and a few edge cases, this is just an internal optimization. (The EventDict
class itself encapsulates several other optimizations as well.)
Event listeners must synchronously prevent their actions. When using an
async
function, the return value is notfalse
but a language-generated promise.
Yes, I understand this.
What is the easiest way to ensure the event is captured […] when you have no direct control over the return value?
True, I have no direct control over the return value, but I do have access to the event object, so the easiest way is to call event.preventDefault()
and event.stopPropagation()
? 😄
I say
m.censor
[(typo)] is more or less the helper you'd write anyways after a point
I can see how such a helper function can be useful, but I don't see anything special in m.capture
if it's literally just m.capture = ev => { ev.preventDefault(); ev.stopPropagation() }
. (Again: not bashing the idea, just wanting to know why m.capture
would be useful/necessary.)
…initially as a higher order function until you found yourself wanting to conditionally disable it more than once or twice
But wait, what do you mean by a higher order function mean in this context? The proposed m.capture
is not a higher order function.
Ah, I guess you mean something like this:
m('button', {
onclick: capture((ev) => {
// ...
}),
}, 'click me')
function capture(handler) {
return ev => {
ev.preventDefault()
ev.stopPropagation()
handler(ev)
}
}
And later when I found myself needing conditional capturing, I'd change the higher order function to a first-order function:
m('button', {
onclick: (ev) => {
if (foo) captureEvent(ev)
// ...
},
}, 'click me')
function capture(ev) {
ev.preventDefault()
ev.stopPropagation()
}
(FWIW I'd probably skip the higher order function.)
Barney's referring to the
elem.onevent
setters and related HTML attributes, of which Mithril's event listener design is based on. Mithril internally usesaddEventListener
, but […] this is just an internal optimization.
Ah, I see. I also see two discrepancies:
elem.onevent
setters and related HTML attributes don't support arbitrary events (like elem.onfoo
), but Mithril does.false
from an event listener attached via elem.onevent
or the related HTML attribute prevents the default action, but doesn't stop propagation – see my previous comment. Mithril does both.@mtsknn
What is the easiest way to ensure the event is captured […] when you have no direct control over the return value?
True, I have no direct control over the return value, but I do have access to the event object, so the easiest way is to call
event.preventDefault()
andevent.stopPropagation()
? 😄
This is true. m.capture
is just meant as a shorthand for that extremely repetitive action.
I say
m.censor
[(typo)] is more or less the helper you'd write anyways after a point
Typo fixed. It's late. 😅
I can see how such a helper function can be useful, but I don't see anything special in
m.capture
if it's literally justm.capture = ev => { ev.preventDefault(); ev.stopPropagation() }
. (Again: not bashing the idea, just wanting to know whym.capture
would be useful/necessary.)
It's not overly special, but also not unique. We already have a couple other relatively small helpers: m.Fragment = "["
for better JSX integration and m.censor
(a glorified _.omit
with an implicitly appended list) for safer component composition and delegation. There's some precedent for tiny helpers, and tiny helpers are the best helpers.
…initially as a higher order function until you found yourself wanting to conditionally disable it more than once or twice
But wait, what do you mean by a higher order function mean in this context? The proposed
m.capture
is not a higher order function.Ah, I guess you mean something like this:
m('button', { onclick: capture((ev) => { // ... }), }, 'click me') function capture(handler) { return ev => { ev.preventDefault() ev.stopPropagation() handler(ev) } }
And later when I found myself needing conditional capturing, I'd change the higher order function to a first-order function:
m('button', { onclick: (ev) => { if (foo) captureEvent(ev) // ... }, }, 'click me') function capture(ev) { ev.preventDefault() ev.stopPropagation() }
(FWIW I'd probably skip the higher order function.)
Yeah, that's the higher order function I was referring to, and you went through the same thought process I did when I abandoned the idea.
Barney's referring to the
elem.onevent
setters and related HTML attributes, of which Mithril's event listener design is based on. Mithril internally usesaddEventListener
, but […] this is just an internal optimization.Ah, I see. I also see two discrepancies:
Yeah I kinda glossed that over in "edge cases", should've listed that explicitly.
- Returning
false
from an event listener attached viaelem.onevent
or the related HTML attribute prevents the default action, but doesn't stop propagation – see my previous comment. Mithril does both.
Good catch. IMHO Mithril's action is the more intuitive one, stopping both.
m.capture
is just meant as a shorthand for that extremely repetitive action.There's some precedent for tiny helpers, and tiny helpers are the best helpers.
Fair enough. 👍
IMHO Mithril's action is the more intuitive one, stopping both.
It's also how jQuery does it. I'm not sure which is more intuitive to me, and it's been ages since I last used jQuery. Personally, I might prefer being explicit and calling ev.preventDefault()
and/or ev.stopPropagation()
manually.
Anyway, Barney talked about "conventionally expected behaviour" and "the way the platform works," but event handlers in Mithril (and jQuery) don't work like native event handlers when it comes to returning false
, so I was wondering if I was missing something. (I don't think I am; it seems to me that conventionally expected behavior might not match the way the platform (native event handlers) works.)
By the way, a third discrepancy (though maybe this too is deemed an "edge case"; after all, you said that Mithril's event listener design is based on elem.onevent
setters and related HTML attributes, not that it tries to mimic the native behavior accurately):
elem.onevent = { handleEvent(ev) {} }
doesn't work. (<button onclick="return obj.handleEvent(event)">
works but is very clumsy and not that different from calling a regular function.)Thanks for the conversation! 😊
Mithril version:
Platform and OS:
Project:
Is this something you're interested in implementing yourself?
Description
And also remove the return value handling logic from this section.
Why
I want to see this idiom die, just as it's largely died elsewhere:
Also, it's not compatible with async functions as event handlers.
Oh, and it'll likely result in a small but measurable net reduction in size compared to what we already do to support
return false
.Possible Implementation
See description.
Open Questions
I think the real discussion is should we continue to support the
return false
that standards bodies have effectively declared as legacy?