leptos icon indicating copy to clipboard operation
leptos copied to clipboard

Fix query params behaviour difference between SSR and Hydrate

Open killertux opened this issue 2 years ago • 6 comments

There is a behavior difference when we get search params (QueryString) right now between the SSR and Hydrate. Not only this, but both cases are sometimes just wrong. For example, if you have any parameter with a '+' encoded into the data it will be converted to a space in SSR. And it seems that Hydrate, can't handle any special character.

So here I am using the crate URL to parse the URL and get the query pairs and convert it to our ParamsMap. I am not sure why we are avoiding using this crate in Hydrate. If there is a good reason we might think of another way.

killertux avatar Jan 20 '23 12:01 killertux

The reason the hydrate version isn't using the url crate is just that the browser already provides an API for URL parsing, and it reduces WASM binary size not to compile and ship a crate that duplicates this behavior. Have you done any testing on the difference in binary size between the current version and your PR? I'd be curious to know what it is. If it's significant, I'd rather find a way to fix the current implementations to make them consistent with one another.

This would be really simple -- just go into an example application, compile it to WASM in the current version, compile it to WASM with your version, and check the difference in binary size.

I'm very open to this if it doesn't increase binary size significantly, as it's always better to be consistent if there's not a cost.

I'd also be curious to see concrete examples

  1. of when there's a mismatch between the browser and server behavior here, and
  2. when you think both implementations are wrong. I might be misunderstanding the issue, but if the url crate and the browser's Url are both different from your expectation in the same way, I'd assume it's your expectation that's wrong

gbj avatar Jan 20 '23 14:01 gbj

The reason the hydrate version isn't using the url crate is just that the browser already provides an API for URL parsing, and it reduces WASM binary size not to compile and ship a crate that duplicates this behavior. Have you done any testing on the difference in binary size between the current version and your PR? I'd be curious to know what it is. If it's significant, I'd rather find a way to fix the current implementations to make them consistent with one another.

This would be really simple -- just go into an example application, compile it to WASM in the current version, compile it to WASM with your version, and check the difference in binary size.

I'm very open to this if it doesn't increase binary size significantly, as it's always better to be consistent if there's not a cost.

I'd also be curious to see concrete examples

  1. of when there's a mismatch between the browser and server behavior here, and
  2. when you think both implementations are wrong. I might be misunderstanding the issue, but if the url crate and the browser's Url are both different from your expectation in the same way, I'd assume it's your expectation that's wrong

I see. It does make sense to keep the wasm size as small as possible.

So to check the size difference and also show the difference in behavior. I create a project using the start template. I added a the following log function so I can log what the message is in both the server side and the client side:

#[cfg(feature = "ssr")]
fn log(message: &str) {
    println!("{message}");
}

#[cfg(not(feature = "ssr"))]
fn log(message: &str) {
    use web_sys::console::log_1;
    log_1(&message.into())
}

So, in the HomePage component, I added the following:

let message = use_query_map(cx).with(move |p| p.get("message").map(|s| s.clone()));
match message {
    Some(message) => log(&message),
    None => {}
};

So I called the application using the following URL: http://127.0.0.1:3000/?message=Data%3A+%24+%26+%2B%2B+7. On the server side, it was logged: Data: $ & 7 On the client side, it was logged: Data%3A+%24+%26+%2B%2B+7 While the expected data would have been: Data: $ & ++ 7

With this PR, both sides log the correct message.

Now about application size. With the code from the main, the total wasm side without any optimizations is 3605831. While with this pr, the total size is 4336344. A 16.8% increase. It seems a little too big so I will play around with this to see if I can make it work using only JS functions.

killertux avatar Jan 20 '23 18:01 killertux

Okay, awesome. Rather than the manual parsing (without decoding) I was doing on the client side, it's probably a much better idea (although awkward in terms of Rust/JS interop) to use URL.searchParams

In JS I tested this out and it parses fine if you use .searchParams as opposed to .search

new URL("http://127.0.0.1:3000/?message=Data%3A+%24+%26+%2B%2B+7").searchParams
> URLSearchParams { message → "Data: $ & ++ 7" }

gbj avatar Jan 20 '23 18:01 gbj

I still want to add some testing to this. But this fixes the issue. And the client wasm size is 3627185 bytes. A slight 0.5% increase

killertux avatar Jan 20 '23 21:01 killertux

Awesome! Yes, that's great and a very small size increase in exchange for the proper behavior. I will let it run the CI for now, and let me know when you're happy with the testing, and I'll merge it.

gbj avatar Jan 20 '23 21:01 gbj

@killertux I just pulled this down and it looks great, and handles special characters very well in query params across the server and client. Good with you if I go ahead and merge it after a final CI run, or did you have any other changes to make?

gbj avatar Jan 21 '23 17:01 gbj

I think that this is good as well

killertux avatar Jan 21 '23 22:01 killertux

Awesome! Merged, and thanks so much for the contribution.

gbj avatar Jan 21 '23 22:01 gbj