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

(#3129) Fix incorrect text escaping during SSR #3381

Merged
merged 6 commits into from
Aug 21, 2023

Conversation

its-the-shrimp
Copy link
Contributor

Description

Fixes #3129

Checklist

  • I have reviewed my own code
  • I have added tests

@github-actions
Copy link

github-actions bot commented Aug 20, 2023

Visit the preview URL for this PR (updated for commit 9949e9f):

https://yew-rs-api--pr3381-fix-ssr-html-escapin-l9hdwp9l.web.app

(expires Mon, 28 Aug 2023 12:15:10 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Aug 20, 2023

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 102.403 102.403 0 0.000%
boids 175.562 175.562 0 0.000%
communication_child_to_parent 95.225 95.225 0 0.000%
communication_grandchild_with_grandparent 108.887 108.887 0 0.000%
communication_grandparent_to_grandchild 105.543 105.543 0 0.000%
communication_parent_to_child 92.701 92.701 0 0.000%
contexts 110.886 110.886 0 0.000%
counter 89.207 89.207 0 0.000%
counter_functional 89.923 89.923 0 0.000%
dyn_create_destroy_apps 92.288 92.288 0 0.000%
file_upload 103.541 103.541 0 0.000%
function_memory_game 174.368 174.368 0 0.000%
function_router 352.161 352.161 0 0.000%
function_todomvc 163.357 163.357 0 0.000%
futures 227.711 227.711 0 0.000%
game_of_life 112.352 112.352 0 0.000%
immutable 188.938 188.938 0 0.000%
inner_html 86.013 86.013 0 0.000%
js_callback 112.996 112.996 0 0.000%
keyed_list 201.200 201.200 0 0.000%
mount_point 89.206 89.206 0 0.000%
nested_list 114.495 114.495 0 0.000%
node_refs 96.166 96.166 0 0.000%
password_strength 1583.162 1583.162 0 0.000%
portals 98.239 98.239 0 0.000%
router 318.168 318.168 0 0.000%
simple_ssr 143.634 143.634 0 0.000%
ssr_router 389.367 389.367 0 0.000%
suspense 118.660 118.660 0 0.000%
timer 91.808 91.808 0 0.000%
timer_functional 100.318 100.318 0 0.000%
todomvc 143.697 143.697 0 0.000%
two_apps 89.919 89.919 0 0.000%
web_worker_fib 154.510 154.510 0 0.000%
webgl 88.529 88.529 0 0.000%

✅ None of the examples has changed their size significantly.

@github-actions
Copy link

github-actions bot commented Aug 20, 2023

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 392.812 393.143 392.943 0.110
Hello World 10 664.030 669.042 665.908 1.288
Function Router 10 2137.381 2162.997 2156.326 8.517
Concurrent Task 10 1006.787 1008.230 1007.339 0.464
Many Providers 10 1514.285 1553.648 1534.814 14.712

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 377.601 378.160 377.787 0.158
Hello World 10 669.347 678.542 674.360 2.518
Function Router 10 2229.834 2278.567 2264.381 13.254
Concurrent Task 10 1006.534 1008.258 1007.315 0.525
Many Providers 10 1587.604 1632.409 1608.026 14.564

Copy link
Member

@ranile ranile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for the PR! I took a cursory look and it looks mostly good to me. Just a few comments

Can you also add a test case for the multi vtext node example I mentioned in #3129 (comment)

packages/yew/src/server_renderer.rs Show resolved Hide resolved
packages/yew/src/server_renderer.rs Outdated Show resolved Hide resolved
packages/yew/src/server_renderer.rs Outdated Show resolved Hide resolved
@ranile ranile requested a review from futursolo August 20, 2023 20:46
github-actions[bot]
github-actions bot previously approved these changes Aug 20, 2023
/// Right now this is used to make `VText` nodes aware of their environment and correctly
/// escape their contents when rendering them during SSR.
#[derive(Default, Clone, Copy)]
pub(crate) enum SpecialVTagKind {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub(crate) enum SpecialVTagKind {
pub(crate) enum VTagKind {

I think it is better to name this as VTagKind as the Other variant is not special.

@ranile ranile enabled auto-merge (squash) August 21, 2023 16:29
@ranile ranile requested a review from futursolo August 21, 2023 16:29
@ranile ranile added bug A-yew Area: The main yew crate labels Aug 21, 2023
@ranile ranile merged commit afde963 into yewstack:master Aug 21, 2023
@ranile
Copy link
Member

ranile commented Aug 21, 2023

Wait why did it think the current user is github-actions...?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In a <style> tag, the selector greater than ">" is escaped into the entity &gt;
3 participants