Move from `return false` to `m.capture(event)` for event handlers

[dead-claudia]

Mithril version:

Platform and OS:

Project:

Is this something you're interested in implementing yourself?

Description

m.capture = event => {
    event.preventDefault()
    event.stopPropagation()
}

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:

m("button", {
    onclick() {
        doStuff()
        return false
    }
}, "bottom text")

Also, it's not compatible with async functions as event handlers.

m("button", {
    async onclick() {
        await doStuff()
        return false // Doesn't work like it looks like it does!
    }
}, "bottom text")

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?

[dead-claudia]

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:

1. Assuming we did something similar for oncreate, it provides an alternative to Make background: true be the default for m.request #2217 in that we could just drop the background option altogether there.
2. This makes it easier for just about everything, including native DOM APIs that return promises like permissions APIs.

[barneycarroll]

@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.

1. With the arbitraryFutureWithSideEffects, data persistence is happening as a side-effect but we're yielding outside of the Mithril idiom, so we take responsibility to redraw.
2. With the 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.
3. In the latter two examples, we want to toggle state immediately and after the asynchronous operation, so we decide that async imperative sequence is preferrable to the return false shorthand, and so rewrite code accordingly. We know that event handler triggers queue a redraw and we know that 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.

[mtsknn]

@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:

• Does not stop event propagation
• Does not prevent the default action with these styles of adding an event listener:
  • $el.addEventListener('click', () => {})
  • $el.addEventListener('click', { handleEvent() {} })
  • <button onclick="handler()"> (no return)
• Prevents the default action with these styles of adding an event listener:
  • <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?

[dead-claudia]

@mtsknn Consider it in light of #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

[dead-claudia]

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?

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.)

[mtsknn]

Event listeners must synchronously prevent their actions. When using an async function, the return value is not false 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 uses addEventListener, 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.
• Returning 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.

[dead-claudia]

@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() and event.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 just m.capture = ev => { ev.preventDefault(); ev.stopPropagation() }. (Again: not bashing the idea, just wanting to know why m.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 uses addEventListener, 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.

Yeah I kinda glossed that over in "edge cases", should've listed that explicitly.

• Returning 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.

Good catch. IMHO Mithril's action is the more intuitive one, stopping both.

[mtsknn]

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):

• Mithril supports event handler objects, whereas 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! 😊