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

Experimental attempt to remove need to have RouteResolver in core #2073

Closed
dead-claudia opened this issue Jan 10, 2018 · 8 comments
Closed
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

@dead-claudia
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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

@dead-claudia
Copy link
Member Author

@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
Copy link
Member Author

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.

@pygy
Copy link
Member

pygy commented Jan 17, 2018

@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.

@dead-claudia
Copy link
Member Author

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
Copy link
Member Author

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 dead-claudia added 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 labels Oct 28, 2018
@dead-claudia
Copy link
Member Author

dead-claudia commented Aug 7, 2019

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 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 #2941 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

2 participants