Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add option to make m.route use m.render instead of m.mount. #1853

Closed
ludbek opened this issue May 17, 2017 · 7 comments
Closed

Add option to make m.route use m.render instead of m.mount. #1853

ludbek opened this issue May 17, 2017 · 7 comments
Labels
Area: Core For anything dealing with Mithril core itself Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Enhancement For any feature request or suggestion that isn't a bug fix

Comments

@ludbek
Copy link
Contributor

ludbek commented May 17, 2017

Problem

Mithril has auto redraw system based upon dom and other events. So one has two different ways to redraw Mithril app
1. dom events
2. m.render

There are situations where one has to skip auto redraw and trigger redraw after a state changes. To me (and new comers) its an overhead to be aware of two different ways to redraw. The auto redraw system is completely redundant with tools like mobx. m.route has great api. Unfortunately it uses m.mount by default.

Solution

It would be great to have an option to make m.route work with m.render.

@dead-claudia
Copy link
Member

@ludbek Could you better clarify what you mean? I'm struggling to see which of these two you're suggesting, if not both:

  1. m.route.set not redrawing.
  2. m.route just rendering on initial load.

Keep in mind you could always use require("mithril/api/router") directly, which is a factory that:

  • Accepts a window and redrawService. You can see which members this requires in this file.
  • Returns an m.route instance (like what you get in the API).

It's an undocumented internal, but for your use case (which isn't idiomatic nor common), you should be okay using it as long as you pin by patch version and pay attention to changes to that file when upgrading.

@ludbek
Copy link
Contributor Author

ludbek commented May 27, 2017

What I mean to say is decouple router from auto redraw system.

@dead-claudia
Copy link
Member

@ludbek So both?

@oldrich-s
Copy link

oldrich-s commented Jul 25, 2017

I have started using Mithril and I love it but I also face this issue. For me it would probably be better if I could completely turn off all the automatic redraws and then I would manually trigger m.redraw() when needed.

A few examples why:

  1. All my m.request calls have background: true because
    a. request calls are inside of onRouteMatch function which also probably redraws
    b. I have multiple requests and only when all succeed I redraw
    c. After a request call, I need to completely refresh including onRouteMatch call using m.route.set(m.route.get(), null, { state: { key: Date.now() } }) so simple m.redraw is not enough
    d. request is inside of onsubmit handler which also redraws
    e. after request I change route by m.route.set which also redraws
    f. after request I open bootstrap modal using jquery => so no redraw needed
    g. after request I open new window using window.open so no redraw needed

  2. All my event handlers such as onsubmit, onchange or onclick have redraw = false because in the handler
    a. I call m.route.set which also redraws
    b. I open bootstrap modal using jquery => so no redraw needed
    c. I call request and then any of the items described in previous bullet point (1.)

  3. Even your own m.route.link has redraw = false inside

So all my requests and event handlers have redraw disabled and still the website works and redraws. That might be sign of unnecessary redraws to me.

Maybe I do something wrong but as it is now all the time I am forced to be aware of automatic redraws and test them to see if multiple redraws occur and if so find the source of these multiple redraws and then to add redraw = false or background: true to the appropriate places. So life would probably be simpler if I could manually trigger redraw and know that no other unexpected multiple redraws happen.

@dead-claudia
Copy link
Member

Related discussions: #2148, #2179 (in subsequent discussion)

@dead-claudia dead-claudia added Type: Enhancement For any feature request or suggestion that isn't a bug fix Type: Breaking Change For any feature request or suggestion that could reasonably break existing code labels Oct 28, 2018
@dead-claudia
Copy link
Member

dead-claudia commented Jul 12, 2019

Fixed in #2458, but I need to update the changelog for it. Edit: ignore this. It's inaccurate.

@dead-claudia
Copy link
Member

dead-claudia commented Jul 12, 2019

I'll note that the way I changed it in #2458 could make it possible to expose a component. One could imagine m(m.route, {redraw = m.redraw, defaultRoute, ...routes}) with the same API as m.route(...), with m.route(root, defaultRoute, routes) being just sugar for m.mount(root, m(m.route, {redraw, defaultRoute, ...routes})). It'd be an easy change to make to the current component-based implementation:

// This is now also a closure component.
var route = function(root, defaultRoute, routes) {
	// At the beginning:
	var redraw = mountRedraw.redraw
	if (root.tag === route) {
		assign(routes = {}, root.attrs)
		redraw = routes.redraw || redraw
		defaultRoute = routes.defaultRoute
		delete routes.redraw
		delete routes.defaultRoute
	}

	// Then the body, just with the few occurrences of `mountRedraw.redraw()` +
	// `mountRedraw.redraw.sync()` replaced with `redraw` and `redraw.sync()`
	// respectively.

	// At the end:
	var inst = {
		// The component currently mounted, with all the current data.
	}
	return root.tag === route ? inst : mountRedraw.mount(root, inst)
}

@dead-claudia dead-claudia added the Area: Core For anything dealing with Mithril core itself label Sep 2, 2024
@dead-claudia dead-claudia moved this to Under consideration in Feature requests/Suggestions Sep 2, 2024
@MithrilJS MithrilJS locked and limited conversation to collaborators Sep 2, 2024
@dead-claudia dead-claudia converted this issue into discussion #2946 Sep 2, 2024
@github-project-automation github-project-automation bot moved this from Under consideration to Completed/Declined in Feature requests/Suggestions Sep 2, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Area: Core For anything dealing with Mithril core itself Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Enhancement For any feature request or suggestion that isn't a bug fix
Projects
Status: Completed/Declined
Development

No branches or pull requests

3 participants