-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Passing additional Props to component #131
Comments
Hi @SaltyDH, thanks a lot for the nice words and for the suggested feature! As others have seen me write in other PRs, one of the main goals of svelte-spa-router is to keep the module as simple as possible. I've resisted feature creeping many times before because I think that a smaller router would have lots of benefits, including:
My rule of thumb is that a feature will be included in the router if and only if one of the two applies:
Looking at your proposed code change, it seems that what you're trying to accomplish is to be able to pass props from the After thinking a bit about it, the same thing could also be achieved using Svelte's stores (or contexts). However, I do want to understand if you've already considered the options above, and if they would not work for your case. Are there benefits in using this approach that should cause us to consider adding this feature as opposed to using an alternative option? |
Hi @ItalyPaleAle thank you for taking the time to consider this proposal. I completely understand where you're coming from and it was part of my thought process in putting together what I hoped was a minimal and considered change to the router. Let me see if I can distil my thoughts and use case down for your consideration. A top level router acts as a seam between logically disparate portions of the application. Typically this separation is desired because it separates routes such as /tasks from /users, two logically disconnected parts of the application. When using the nested routers feature that this router supports, this paradigm is the polar opposite, two logically connected components that are divided by a seam (the router) e.g. /tasks/:taskId/overview and /tasks/:taskId/details. To me, it makes a lot of sense to use the router in this situation because of the features provided by the router including consistent url history, and synchronisation with the address bar etc, guard etc. In terms of stores and contexts:
In contrast, the suggestion I have put forward:
Hopefully my thoughts resonate with you and you may consider these suggestions for a future update. Kind regards. |
Sorry for the delay in responding… Thanks for your thoughtful response. I agree with your point and I'm going to accept this. Would you like to open a PR (to get proper credits)? |
Fixes #131 Credits: @matt-psaltis
@matt-psaltis this is implemented in the 3.0 branch and will be released shortly. Thanks for the contribution! One note: because the props are passed to every single route, if the route doesn't ask for them, you'd see a warning similar to |
Hi @ItalyPaleAle sorry I've been unresponsive to this I would have liked to contribute with the pull request, thank you for doing so on my behalf. With this change, it looks like the props are exposed as part of the routes configuration and I've successfully been able to add them via routes. One difference in the original suggestion was having the props variable exported so that it can be supplied directly on the router component from the parent component rather than via the routes config. Just curious if you can spare a moment :) thanks again! |
Hey @matt-psaltis thanks again for the contribution and for the convincing argument above! I initially merged the change just as you proposed above. However, after some testing, I decided to alter the design for two reasons.
I know that the new API design is a bit more complex than just passing a prop to the router component... But I hope that the gains in flexibility (and the less annoying warnings) will compensate for the minor additional annoyance of having to use |
Thank you for making the only Svelte router that has just worked. A huge bonus - it matches the documentation! :D
I was hoping for a change to the Router component to be able to pass additional props to the rendered component.
Given a router definition the inclusion of the {props} attribute would allow arbitrary data to be passed to the rendered component.
Changes required:
Rendered component can now access these props in the standard way using
export let id;
As per: https://svelte.dev/repl/74593f36569a4c268d8a6ab277db34b5?version=3.12.1
Appreciate your consideration on this additional feature.
The text was updated successfully, but these errors were encountered: