Alternative request/redraw integration

[barneycarroll]

This is a proposal relating to #1442. It allows redraw: false to be passed as an option to m.request when using bundled Mithril in a way that addresses @lhorie's concern that this would on the face of it be too tight a coupling between the request API to the render API - this does it in such a way that request is actually more generic, and isolates the request/redraw hooks to the bundling module.

It's difficult to write tests for this feature - I'm stymied by the fact that AFAICT I'd need to invoke both the piecemeal xhr & mock modules to test the bundled module - and I'm not sure how to do that.

Having said that, there were no tests for request/redraw integration in the first place, and this does pass all existing tests.

[tivac]

This doesn't look like the behavior quite matches what it's replacing.

Currently m.request wraps the Promise being returned so that the .then() function will always be wrapped and able to invoke whatever the completion callback is after the entire promise chain completes.

Your version looks like it'll call m.redraw() as soon as the request finishes, before any other .then() are run.

[barneycarroll]

My understanding is that that isn't an issue, since the redrawService is subscribed to in m.mount, which always defers the actual recomputation by some value of time.

Sequential Promises chaining off the request will therefore always resolve before the actual view recomputation. Right?

[tivac]

So after a bunch of discussion in gitter (starting about here) I think we ended up in agreement.

• The automatic redraw should happen at the end of the entire Promise chain
• Other redraws invoked from code should not be blocked during the wait for the resolution of the Promise chain. The current implementation does this and it's not desired.

@barneycarroll @lhorie

[barneycarroll]

@tivac yep, AFAIC this patch fulfills these criteria.

[leeoniya]

out of curiosity how do you guys determine the end of a promise chain? if m.request returns a promise how can it know when the chain is "done" presumably to chain a redraw to it?

[tivac]

@leeoniya The previous implementation wrapped the .then() method returned by the Promise.

[leeoniya]

thanks, i suspected it wasn't a bare es6 promise. i understand how you can auto-chain the redraw. but still not how you can determine that the chaining is completed without an explicit external .end() call or similar. it seems like something stream.end() subscribers can do though...

[lhorie]

@barneycarroll this PR doesn't work; it redraws too early (run npm run dev and try ThreadItJS example in your folder)

[barneycarroll]

This raises a lot of awkward issues :/

@leeoniya is right, this relies on a decorated promise implementation and assigns dubious semantics to it. A promise can be forked, chained, passed to other contexts. A receiving context from one of these forks may well sit on that promise reference until an arbitrary condition resolves and only then declare a dependency upon it. The idea that Mithril can determine when the chain ends is fallacious.

The magic interpretation of intent involved here is too high: either a request should always trigger a redraw on resolution, or never. Anything in between is too difficult to reason about, and therefore not a viable convenience feature.

I think the desired intent of this patch is good - ie, you will get an async redraw as a result of requests and event handlers settling. Advanced users can opt out, after all - in component code or by building their own non-auto Mithril (at their own peril). What do we reckon?

[dead-claudia]

@barneycarroll This is why I don't want m.request to have any redraw semantics whatsoever. It's too magical to reliably implement, even with a custom static compiler.

There's always promise.then(m.redraw), which is maybe one extra line tops.

[lhorie]

it works like this now: https://github.com/lhorie/mithril.js/blob/rewrite/request/tests/test-request.js#L301-L317

Standalone has no redraw semantics