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

m.request: Add options with holdRender attribute #62

Closed

Conversation

sdemjanenko
Copy link

By default we will hold rendering while the xhr is running. Sometimes
users will not want render to block while the xhr is running. This
option will give users control of this behavior.

By default we will hold rendering while the xhr is running.  Sometimes
users will not want render to block while the xhr is running.  This
option will get them control of this behavior.
@lhorie
Copy link
Member

lhorie commented Apr 29, 2014

Did calling m.redraw() from the controller not work for you? You mentioned in #52 that you're getting a different controller instance when you call it, but AFAIK, Mithril only reinstantiates controllers if you call m.module, either directly or implicitly via m.route. If Mithril is instantiating controllers in other cases, then that might be a bug.

I don't want to add this as an option of m.request because m.request is a model layer tool and that layer should not dictate (or care about) the rendering profile of the module/controller that is calling it.

@sdemjanenko
Copy link
Author

m.redraw() didn't work for me. My workaround was using _.defer. However if was change the route while waiting for m.request to return then the UI would not rerender until the xhr finished (note that the xhr may not even be used on the new route). It seems kinda unfortunate that one request out of potentially many could block rendering (which makes progressive rendering impossible).

I agree that m.request is a model layer tool and shouldn't care about rendering. But isn't calling m.start/endComputation() affecting the rendering profile anyway?

@sdemjanenko
Copy link
Author

I think if m.redraw were to do the requestAnimationFrame that function redraw does that would solve my problems.

@lhorie
Copy link
Member

lhorie commented Apr 29, 2014

Ooh, I understand the use case now. This is a really good point, it's a scenario I had not thought about. The use case for the abort() request feature is also clearer to me now. Let me fiddle around with that and I'll get back to you. I just want to make sure I get the API right.

Re: your workaround, _.defer (or setTimeout / setImmediate and friends) do remove the m.request from the Mithril computation chain, so yes that would work (though I'd rather get this scenario working properly than suggesting that everyone running into your issue use that workaround).

@lhorie lhorie mentioned this pull request Apr 29, 2014
@lhorie
Copy link
Member

lhorie commented Apr 30, 2014

I thought about this for a bit and I've implemented a slightly different option called background. This is in origin/next and scheduled to be released in v0.1.11. Docs and tests have also been updated.

When set to true, this option completely disables the request's ability to interfere with rendering.

This is useful for things like background syncing. For your specific case, you'd have to manually call m.redraw at the end (assuming redrawing at that point is desired)

m.request({method: "GET", url: "/foo", background: true})
  .then(m.redraw)

@lhorie lhorie closed this Apr 30, 2014
@sdemjanenko sdemjanenko deleted the m_request_hold_render branch May 5, 2014 17:21
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

Successfully merging this pull request may close these issues.

2 participants