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

Experimental attempt to remove need to have `RouteResolver` in core

Open dead-claudia opened this issue 6 years ago • 8 comments

People have been having weird issues and pretty specialized needs that route resolvers alone can't deal with. Also, I managed to reduce it to using only a single extra, trivial primitive that simply redirects to the default route (without having to trigger a hash change or anything like that).

dead-claudia avatar Jan 10 '18 17:01 dead-claudia

So if I could get access to that primitive, I could factor the entire mess out of core and into a third-party library. (Smaller core is always a plus IMHO.)

dead-claudia avatar Jan 10 '18 17:01 dead-claudia

This came as a product of a Gitter conversation with @barneycarroll, BTW.

dead-claudia avatar Jan 10 '18 17:01 dead-claudia

@pygy and @tivac, what are your thoughts on this? Also, is there a chance this could ride the v2 train? (The requisite method would be implemented and shipped on v1 as well, but the removal would have to wait for v2.)

Oh, and the relevant Gitter conversation took place here.

dead-claudia avatar Jan 17 '18 12:01 dead-claudia

Alternatively, we could provide a method to just get the default route, and I could just use m.route.set(m.route.default(), null, {replace: true}) in the promise rejection handler. That would be even simpler.

dead-claudia avatar Jan 17 '18 12:01 dead-claudia

@isiahmeadows this is not equivalent to RouteResolvers:

It will not preserve layouts on route change, and will not keep the previous endpoint live while the async route is pending.

pygy avatar Jan 17 '18 17:01 pygy

It will not preserve layouts on route change, and will not keep the previous endpoint live while the async route is pending.

Ugh...I made a few edits to make it wrap m.route instead so I could do that correctly.

dead-claudia avatar Jan 17 '18 23:01 dead-claudia

An easier alternative would be to make every route an attrs => vnode render function instead, and although it'd be breaking, I could easily make a migration helper that wraps m.route, with special logic for closure components (wrapped with identity preserved for routes) so you could migrate to vnodes and so I don't double-construct them pre-migration. (That's probably the only non-trivial bit.)

As for route resolvers, with render functions (instead of render components) whose vnodes are diffed on route change, that would make my bit a little easier - I could write it as a simple component instead, without having to wrap anything. It'd also have benefits for those using common base layout components, as those would simply diff, and they wouldn't need to wrap m.route to use them. (Or, TL;DR: they scale better, and less boilerplate is always a plus.)

dead-claudia avatar Jan 17 '18 23:01 dead-claudia

Update: no m.route.default() is needed - this line right here is basically m.route.set(defaultRoute, null, {replace: true}), and it probably should be that.

Edit: So a userland wrapper would just call m.route and save the default route to redirect to later.

However, we would want to move m.route.SKIP to work with render as well, and this would allow the full RouteResolver API to be implemented in userland.

dead-claudia avatar Aug 07 '19 13:08 dead-claudia