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

WIP - Using Svelte Context API's for deeply accessible helpers & router state #16

Closed
wants to merge 3 commits into from

Conversation

matthewrobb
Copy link

@matthewrobb matthewrobb commented Nov 30, 2019

The inspiration for this PR came from the issue I detailed in #15. The changes are as follows and would likely require updates to documentation etc.

The rollup plugin now generates as 'svelte-filerouter/routes'. Most of the logic in navigate.js has moved into store.js and is now fully reactive by default. The generated routes module now looks something like:

import * as router from 'svelte-filerouter';

export const routes = [...];
export const options = {...};

router.options.set(options);
router.routes.set(routes);

Documentation would need to be updated to state that your application needs to import 'svelte-filerouter/routes' in order to for routing to work.

@matthewrobb
Copy link
Author

@jakobrosenberg Looking for feedback as to whether or not this is in conflict with the general direction of the library?

@jakobrosenberg
Copy link
Member

@matthewrobb any chance this can be done without the use of store? Or can you test that it doesn't fall victim to this bug?
sveltejs/svelte#3685

I'm planning a v2 and consider to use the url helper strictly as a prop, so it can be context aware.
#19

@matthewrobb
Copy link
Author

@jakobrosenberg This can definitely be done without a store. I think the store solution is elegant but in the light of that bug I will rework it to go a different way. That's a very unfortunate bug.

@jakobrosenberg
Copy link
Member

Yes, horrible bug. Speculating, I'd say that most people who use transitions aren't aware of the bug's cause and simply end up disabling their transitions. At least that's what I've observed on discord. (Are you on there by any chance?)

What do you think about simply using setContext for all (except user defined) props in v2?

@matthewrobb
Copy link
Author

@jakobrosenberg I think using setContext for most of this stuff is perfect and any props passed into Router at the root can easily get passed to routed components like so:

<svelte:component {...$$props} />

@matthewrobb matthewrobb changed the title WIP - Eliminating circular dependency warnings and other cleanup WIP - Using Svelte Context API's for deeply accessible helpers & router state Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants