mithril.js icon indicating copy to clipboard operation
mithril.js copied to clipboard

Make `background: true` be the default for `m.request`

Open dead-claudia opened this issue 7 years ago • 13 comments

Considering how (not) often it is you want to request data and immediately re-render, it'd make more sense to just background everything by default and require an explicit background: false to force a re-render. If this turns out successful, we might want to consider removing the option altogether in the future (post-v2). We're kind of the exception in trying to provide some sort of autoredraw mechanism from network requests, and it's not like they couldn't drop a .then(m.redraw, m.redraw) at the end if necessary.

dead-claudia avatar Aug 28 '18 12:08 dead-claudia

I find the a redraw required after 90% of my http requests. I prefer to keep the flag for the less-common option. Consider your usage, how more often then not do you @isiahmeadows not require a redraw?

yossicadaner avatar Aug 29 '18 13:08 yossicadaner

I've never been hurt by the extra redraw, and I have relied on it unconsciously multiple times.

I almost always wrap m.request anyway, so I would find it fairly simple to disable the functionality generally in a project if needed (I haven't had that need yet).

Just a bit of anecdata from here ;)

porsager avatar Aug 29 '18 13:08 porsager

@yossicadaner Usually, I do require an eventual redraw, but it's usually something where an explicit redraw is better. If you need to do request multiple things or if it's a more complex request, you'll have to do an explicit redraw either way, so the extra redraw is wasted.

There's also a little bit of philosophy, where I noted in Gitter that removing the autoredraw behavior would fully decouple the request utility from the rendering. (That'd make it easier to test and more usable server-side, assuming an XHR mock.)

dead-claudia avatar Aug 29 '18 19:08 dead-claudia

Me and Isiah chatted about this on Gitter. The proposal isn't a feature, it's a design proposal that would allow Mithril's constituent modules to be separated by default such that the renderer & request modules could be imported individually. Auto redraw could then be reimplemented as a wrapper.

barneycarroll avatar Aug 29 '18 21:08 barneycarroll

Ah, for some reason I had the idea mithril returned a special promise that would hold the redraw until the end of the promise chain. Just tested, and I see that's not the case, so I'm actually fine with background being removed and just having it in a wrapper if necessary.

Maybe I remember that end of chain thing from back when m.request returned a stream?

porsager avatar Aug 30 '18 07:08 porsager

@porsager Leo and Pat did work at that in the initial rewrite. The problem with that proposal was that it misunderstands the nature of Promise chains: you can't determine where they end; promise chains can be infinitely recursive.

Discussion here.

barneycarroll avatar Aug 30 '18 07:08 barneycarroll

@porsager You're probably remembering from v0.2, where you'd see some very subtle behavior that looks like that:

  • m.deferred's .then callbacks were synchronously invoked.
  • m.deferred returned this really weird magic m.prop-like object that was somewhere between a prop and a promise, kind of both, kind of neither.
  • m.request scheduled a re-render using setTimeout

Thanks to a fully synchronous pipeline and it waiting until the whole pipeline finishes for most calls, it was pretty easy to be a bit magical about it and still get away with it for the most part. (The few glitches are precisely why background: true existed, but it wasn't there from day 1 IIRC.) Back then, it was still pretty easy to use, even with the async pipeline, since:

  • value: ctrl.value(), oninput: m.withAttr("value", ctrl.value) was a very common idiom, and is precisely why I've been pushing a little to bring that back into the core bundle.
  • m.request returning a m.prop-like instance made it interact very nicely with the above - you could fetch a resource with an initialValue: ..., and use that within the view for a very nice and easy fetch-and-two-way-bind combo without much effort.

But with v1+ returning an actual promise, the sync/async timing schizophrenia around m.redraw (fixed in v2), and the fact the chain recursively overrides .then for all its magic semantics instead (it doesn't even do that correctly - try returning a primitive like undefined), I'm surprised nobody has reported issues with it yet.

dead-claudia avatar Aug 30 '18 07:08 dead-claudia

Ah yes, that's what I was thinking of @barneycarroll ;)

porsager avatar Aug 30 '18 07:08 porsager

Isn't this the case now @dead-claudia?

StephanHoyer avatar Feb 24 '22 12:02 StephanHoyer

Ah now I get it. Sorry for the noise. Should have read more carefully.

Current

  • Autoredraw after XHR-Resolve

Future

  • Manual redraw (or auto-redraw by a wrapper) after XHR-Resolve

Goal: decouple request from redraw

StephanHoyer avatar Feb 25 '22 17:02 StephanHoyer

I think it's worth asking: is a Promise chain the more-common use case?

Considering how (not) often it is you want to request data and immediately re-render, it'd make more sense to just background everything by default and require an explicit background: false to force a re-render.

@dead-claudia I disagree with your original premise that an immediate re-render is rare, but the statement itself is ambiguous: does "immediately" here refer to the moment after the request is sent, or to the moment data is returned? In the former case, a UI which doesn't acknowledge that a request has been made on a user event is typically very poor indeed. So if this is a background request, independent of user events, then background: true seems perfectly ergonomic. In the latter case, I return to my question: do we think it's most common to perform multiple async operations before updating the UI?

I would contend that the batteries-included approach is the easiest to grok. Adding background: true to chained cases seems more the Mithril way.

There's also a little bit of philosophy, where I noted in Gitter that removing the autoredraw behavior would fully decouple the request utility from the rendering. (That'd make it easier to test and more usable server-side, assuming an XHR mock.)

Mithril is a UI library above all else. I don't see that decoupling the http sugar from the renderer makes any sense, unless we want to pull it from core entirely, which is a whole other discussion.

CreaturesInUnitards avatar Feb 26 '22 18:02 CreaturesInUnitards

Returning to this with a deep sense of guilt for my failure to provide necessary help when it was needed.

But: Mithril delusions about the nature of deferred payloads are my greatest demon and I must defeat these creatures wherever I find them.

Promises are intrinsically ideologically screwed but that’s not something Mithril ought to fix or fake or solve. The one thing that could be valuable is a callback / promise resolution after success.

In 2016 we were trying to re-invent asynchrony. All that matters to hackers and innocent users alike is the notice of completion: just expose that. Everybody who really cares about the particulars wants this mechanism, and the naïve youngsters will be glad to be on the same footing (eventually). There is no glory in making it more complicated.

If we expose a generic redraw completion promise, all these ambiguous problems disappear.

barneycarroll avatar Feb 26 '22 18:02 barneycarroll

@CreaturesInUnitards @barneycarroll @StephanHoyer I'm also looking at stuff like https://github.com/MithrilJS/mithril.js/pull/2428 (explainer), especially in light of language proposals like https://github.com/tc39/proposal-rm-builtin-subclassing and https://github.com/tc39/proposal-eventual-send. Anything that mucks with .then behavior or microtask scheduling can break us (and already has once), and I want to us to not be stuck in that kind of situation.

Revisiting this years later, I'd be fine with it if we change it to an optional onresponse + onerror pair that auto-redraws (and settles the outer returned promise accordingly) once the related hook settles. The real core of the issue here isn't whether it redraws, but lack of clarity on when it redraws.

I've grown weaker on the philosophical concerns, but my predictability concerns (why I still won't use background: false personally) still remain and that would address them.

dead-claudia avatar Feb 26 '22 21:02 dead-claudia