-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add support for scoped contexts #48
Conversation
There are some serious pitfalls using global (thread local) state while in SSR time. To avoid major security risks, I've added a strict panic if this is attempted. Also improve api clarity.
While this can be a security issue, there are some use cases where it's perfectly viable to utilize thread-local state on the server. This is a case where good documentation should help with the niche pitfall, instead of forbidding it entirely.
I just went by this PR and I really like it and would also help me for specific cases. I could use them instead of the yew OOTB use_store hooks, which unfortunately have no reducer functions.... Ebenso I like the new feature I'm strongly argue to not change the actual api for the Dispatcher. This would break my code at 300+ places... let dispatch = Dispatch::<Counter>::new(&cx); I would rather propose to keep the new function as it is and have an additional function like this: let dispatch = Dispatch::<Counter>::ctx(&cx); I also don't understand the benefit of an App vs Thread context. But this is definitely on me. I hope, this feedback helps :) |
Only wasm32 targets are allowed global context access. Accessing global context in a multi-threaded environment is unsafe, and not allowed.
This does break a lot of code, but is way more descriptive and clear as to what exactly is happening. Since we are still pre-release, breaking changes are expected, and shouldn't inhibit improvements.
@Roba1993 thanks for the feedback!
Hopefully you can refactor easily with a little regex magic. We aren't 1.0 yet, so breaking changes are to be expected, and shouldn't get in the way of improving the API. I think this new naming scheme makes it super clear to the user when exactly they are accessing global context. It's a topic that may introduce confusion to newcomers, and so vitally important we make it as easy to understand as possible. |
Includes a workaround for rust-lang/rust#67295 Global context is only available for wasm, but tests are run natively. Because #[cfg(doctest)] does not work as expected, we include a feature to fill that role. We should eventually switch back over when the fix is merged into cargo.
Yes, I will be able to do this 😄 |
Primarily important for SSR support, containing state to the scope of the app, instead of the thread.
For a basic example, you can now create a local context for your dispatch:
Changes to one context are not reflected in any others:
The default context for a dispatch is global (thread local).
A root component is now available to provide context that is scoped to the app:
Hooks will detect the context provided by
YewduxRoot
. If no root is provided, it uses global by default.