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

Support implementing Route on structs #244

Closed
arctic-hen7 opened this issue Sep 16, 2021 · 6 comments · Fixed by #434
Closed

Support implementing Route on structs #244

arctic-hen7 opened this issue Sep 16, 2021 · 6 comments · Fixed by #434
Labels
A-router Area: router C-enhancement Category: new feature or improvement to existing feature

Comments

@arctic-hen7
Copy link
Contributor

Currently, Sycamore explicitly states that the Route trait is for enums, and as such its method match_route doesn't take the self parameter. However, there are use-cases (such as Perseus) for which routes may be stored in a struct, and can only be accessed through the self parameter. I've explored global variables and contexts as possible solutions to this problem, but the former ends up requiring generation of core types and the latter becomes quickly infeasible due to the nature of reactive scopes.

Hence, I would propose that the Route trait's methods take the self parameter so that they can be feasibly implemented on structs as well as enums. In theory, I would think this should be relatively simple, especially because the rest of the user-facing interface should be able to stay exactly the same, though some changes would be needed to the derive macro for Route.

@arctic-hen7 arctic-hen7 added the C-enhancement Category: new feature or improvement to existing feature label Sep 16, 2021
@arctic-hen7
Copy link
Contributor Author

Also, this technically wouldn't need to move Sycamore to v0.7.0:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

Quoted from Semantic Versioning spec item 4.

@lukechu10 lukechu10 added the A-router Area: router label Sep 16, 2021
@arctic-hen7
Copy link
Contributor Author

arctic-hen7 commented Sep 16, 2021

As I look at the code in further detail, I'm seeing that this would require significant changes to the routing model, and would only be suitable as a longer-term feature.

@arctic-hen7
Copy link
Contributor Author

For anyone else encountering this problem, I advise you generate an enum with the data you need it to have using a macro if possible. It sounds atrocious, but it actually works quite well if you move as many functions as possible out of the macro!

@lukechu10 if you think this more flexible routing model is worth pursuing in the longer-term for albeit a very niche use-case (framework building), please re-open this issue! Otherwise, the solution above will be sufficient for now I think (for me at least).

@lukechu10
Copy link
Collaborator

I think the best way to solve this issue is to provide a way to opt-out of the Router attaching a click handler.
This way, Perseus can attach its own handler and not have any conflict at all with Sycamore's. You would still be able to use sycamore's routing infrastructure by manually calling navigate instead.

@arctic-hen7
Copy link
Contributor Author

That may be good long-term, but I don't think that's the crux of the issue anymore. As described here, the principle issue appears to be that Sycamore executes Perseus' routing logic properly, but then doesn't continue on to run the closure provided to the Router for some reason. I'll start attempting to debug this tomorrow and see if I have any luck, but I think the routing infrastructure seems to be fine as is, it just seems to be doing some kind of overly-aggressive caching.

@arctic-hen7
Copy link
Contributor Author

I'm now trying to remove the magic from Perseus, and I've found myself back at this problem! The issue is that I need to be able to get three Perseus properties (the render config, the templates, and the locales) into the logic the match_route function runs. The old system was reacquiring all these by using a macro to generate the enum, which is now infeasible with the new design.

It would be really useful if Sycamore could support taking the &self parameter to match_route, which would allow using struct Routes that contain custom information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-router Area: router C-enhancement Category: new feature or improvement to existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants