Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: I think we should get rid of Mithril's deferred #947

Closed
mikegleasonjr opened this issue Jan 24, 2016 · 1 comment
Closed

Discussion: I think we should get rid of Mithril's deferred #947

mikegleasonjr opened this issue Jan 24, 2016 · 1 comment

Comments

@mikegleasonjr
Copy link

And let the client choose it's own shim/implementation of Promises if at all if he want to use it client side.

Short explanation

  • The only place they are used is with m.request
  • It does not follow the A+ standard (see why below)
  • Trying to use it as a replacement of real Promises elsewhere brings headache
  • We mix and match m.deferred and m.prop, this is confusing for me but not my main pain point

Long explanation

The main reason I see why it does not follow the standard regarding resolving in-sync instead of async is in regard to the redrawing mechanism. The then handlers of the deferred returned by m.request have to be executed in-sync for the library to be able to redraw right after their execution:

Mithril's way:

theDeferred.resolve("result");
m.endComputation();

is executed in order. The then handlers of the deferred, before m.endComputation. So the page is redrawn ok.

Promises way

p.resolve("result");
m.endComputation();

m.endComputation() would be executed before the handlers registered on the promise. So the page is redrawn first, then possibly modified by the handlers... not good.

So I think the tool for the job here is simply a regular callback to the m.request function:

m.request = function(opts, callback) {
  [...]
  callback(null, result);
  m.endComputation();
  [...]
}

I know I know it's a huge thing that would break features like http://mithril.js.org/mithril.request.html#rendering-before-web-service-requests-finish

But I don't know, it seems to bring so much confusion for me elsewhere else.

That would also be more transparent on how things are re-rendered. Plus, if we decide to go in the direction explained here #936, clients could call m.redraw.strategy("x") to change the redraw strategy just like DOM event handler, further unifying the whole redraw api. But that's another thing.

An example of an obscure complexity with deferred is this: Let's say I keep a copy of the promise returned by m.request and call then on it after a second. The page won't get redrawn. I have to read the source to find that out. In other words, the then callback chain have to be registered on the promise before m.request resolves it for the redrawing system to work.

Backward compatibility

We would have problems since we would change the api and we use deferred as m.prop as I said.

All in all Mithril is an amazing library but I get irritated by the fact that we, as clients, don't (1) have an unified api to work with the redrawing system (hence #936) and (2) there are leaks on the client side because of the needs of the internals:

For example,

  1. With m.request we play with the background flag to change the redraw behaviour
  2. With DOM event handlers & route change, we call m.redraw.strategy("x") to change the redraw behaviour
  3. With our own services, if we have sync code, we have to do funky things to be able to provide deferreds like:
func getX() {
    var deferred = m.deferred();
    m.startComputation();
    setTimeout(function() {
        deferred.resolve(cached);
        m.endComputation();
    }, 0);
    return deferred.promise;
}

Why do I return a deferred if my function is sync? The scenario is more complicated, I have an async 3rd party but I cache similar calls. So on a cache hit, the calls are sync and async otherwise. I also call the compute methods to try to have a unified way of doing things in both cases.

I don't know it seems m.request tries to do too much.

@mikegleasonjr
Copy link
Author

Closing since I talked about this on the channel. I will fork and remove the auto-redrawing system as well as deferred from Mithril altogether. If anyone is interested, I can explain my choices somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant