-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update Yew Router as per #2113 #2118
Conversation
# Conflicts: # packages/yew-router/src/components/redirect.rs # packages/yew-router/src/lib.rs
Visit the preview URL for this PR (updated for commit 4621208): https://yew-rs--pr2118-yew-router-proposal-nl2t2n44.web.app (expires Thu, 28 Oct 2021 11:16:49 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
packages/yew-router/src/utils.rs
Outdated
// #[derive(Serialize, Clone)] | ||
// struct QueryParams { | ||
// foo: String, | ||
// bar: u32, | ||
// } | ||
|
||
// #[test] | ||
// fn test_get_query_params() { | ||
// assert_eq!( | ||
// parse_query::<HashMap<String, String>>().unwrap(), | ||
// HashMap::new() | ||
// ); | ||
|
||
// let query = QueryParams { | ||
// foo: "test".to_string(), | ||
// bar: 69, | ||
// }; | ||
|
||
// yew_router::push_route_with_query(Routes::Home, query).unwrap(); | ||
|
||
// let params: HashMap<String, String> = parse_query().unwrap(); | ||
|
||
// assert_eq!(params, { | ||
// let mut map = HashMap::new(); | ||
// map.insert("foo".to_string(), "test".to_string()); | ||
// map.insert("bar".to_string(), "69".to_string()); | ||
// map | ||
// }); | ||
// } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these tests commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parse_query
and push_route_with_query
are replaced by location.query()
and history.push_with_query()
.
You can find the updated test here: https://github.com/yewstack/yew/pull/2118/files#diff-ec64ea4b4e8cbcfcfc72c33eec28309e23f5f32be356f9a0ebc6068c1938d936R41
I just forgot to remove the old test, will do shortly.
Description
Fixes #2113, #2116
Supersedes the first section of #1860.
This PR mainly makes
yew-router
a little bit more likereact-router
, it consists of the following changes:History
andLocation
traits to makeyew-router
SSR capable (MemoryHistory to be added.).history
location
androute
.RouterScopeExt
trait to provide a way to accesshistory
,location
androute
and add listeners for struct component.yew_router::service
(Replaced by hooks andRouterScopeExt
).<Router />
a context provider and Add<Switch />
component to dispatch between routes.<Redirect />
component that mirrors the behaviour ofreact-router
's<Redirect />
component.route
prop onLink
component toto
.Default
if#[not_found]
is present onRoutable
.Things to be decided:
The current suggestion is to use
Yewdux
instead (mainly due to ergonomic concerns).However, it's possible to push multiple states with the same route but state differs which can be hard to distinguish if separated from history and it's easy to implement so I added them anyways (and using
ctx.link().history().state()
should not be more difficult thanWithDispatch
ergonomic wise).Location
Most setters on location would cause the page to reload and
history
package does not implement a setter on them.Not sure if we should follow this behaviour.
For now I have opted for
history
package's behaviour which is making location to be read only.Checklist
cargo make pr-flow