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.

Sign up for GitHub

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

Jump to bottom

Change the #[function_component] attribute to #[component] by default #3447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Change the #[function_component] attribute to #[component] by default #3447

wants to merge 2 commits into from

Conversation

tdooms
Copy link

@tdooms tdooms commented Oct 5, 2023 •
edited
Loading

Description

Change the #[function_component] attribute to #[component] by default (the original name is kept as a re-export for backwards compatability).

With the current state of function components, it is no longer required to have a verbose name.

All occurences in the examples, docs and internal use have been changed to #[component].

Checklist

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

@github-actions
Copy link

github-actions bot commented Oct 5, 2023 •
edited
Loading

Visit the preview URL for this PR (updated for commit 3238bb5):

https://yew-rs--pr3447-components-315ai2yg.web.app

(expires Thu, 12 Oct 2023 12:12:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Oct 5, 2023 •
edited
Loading

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 382.319 382.690 382.477 0.098
Hello World 10 773.400 776.621 774.229 1.021
Function Router 10 2459.513 2476.405 2469.237 6.634
Concurrent Task 10 1009.122 1011.866 1010.663 0.813
Many Providers 10 1750.867 1788.010 1766.883 12.450

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 382.374 384.419 382.760 0.623
Hello World 10 776.463 780.503 778.232 1.428
Function Router 10 2459.141 2475.647 2470.558 5.410
Concurrent Task 10 1008.201 1011.741 1010.204 1.203
Many Providers 10 1748.473 1787.680 1765.321 13.563

@github-actions
Copy link

github-actions bot commented Oct 5, 2023 •
edited
Loading

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 102.945 102.945 0 0.000%
boids 175.666 175.666 0 0.000%
communication_child_to_parent 95.298 95.298 0 0.000%
communication_grandchild_with_grandparent 109.037 109.037 0 0.000%
communication_grandparent_to_grandchild 105.718 105.718 0 0.000%
communication_parent_to_child 92.784 92.784 0 0.000%
contexts 113.453 113.453 0 0.000%
counter 89.197 89.197 0 0.000%
counter_functional 89.931 89.931 0 0.000%
dyn_create_destroy_apps 92.315 92.315 0 0.000%
file_upload 103.514 103.514 0 0.000%
function_memory_game 174.629 174.629 0 0.000%
function_router 353.632 353.632 0 0.000%
function_todomvc 163.455 163.455 0 0.000%
futures 227.446 227.446 0 0.000%
game_of_life 112.217 112.217 0 0.000%
immutable 188.789 188.789 0 0.000%
inner_html 85.981 85.981 0 0.000%
js_callback 113.457 113.457 0 0.000%
keyed_list 201.198 201.198 0 0.000%
mount_point 89.188 89.188 0 0.000%
nested_list 115.762 115.762 0 0.000%
node_refs 96.291 96.291 0 0.000%
password_strength 1720.994 1720.994 0 0.000%
portals 98.362 98.362 0 0.000%
router 319.566 319.566 0 0.000%
simple_ssr 144.239 144.239 0 0.000%
ssr_router 391.404 391.404 0 0.000%
suspense 119.066 119.066 0 0.000%
timer 91.797 91.797 0 0.000%
timer_functional 100.473 100.473 0 0.000%
todomvc 143.688 143.688 0 0.000%
two_apps 89.897 89.897 0 0.000%
web_worker_fib 138.868 138.868 0 0.000%
web_worker_prime 190.462 190.462 0 0.000%
webgl 88.552 88.552 0 0.000%

✅ None of the examples has changed their size significantly.

@tdooms tdooms changed the title Change the #[function_component] macro to #[component] by default Change the #[function_component] attribute to #[component] by default Oct 5, 2023
@ranile ranile added A-yew Area: The main yew crate A-yew-macro Area: The yew-macro crate labels Oct 5, 2023
futursolo
futursolo approved these changes Oct 22, 2023
View reviewed changes
Copy link
Member

@futursolo futursolo left a comment

Choose a reason for hiding this comment

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

I am in favour of this change for a shorter name for function component.

Thanks!

ranile
ranile reviewed Oct 22, 2023
View reviewed changes
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.

Sorry it took me so long to finish this review. I like this change. There are files/lines that aren't changed but still show up as changed. Can you see what's up with those?

packages/yew-macro/tests/function_component_attr/re-export-pass.rs
@@ -0,0 +1,12 @@
#![no_implicit_prelude]
Copy link
Member

Choose a reason for hiding this comment

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

Can you use a failing test for this so the deprecated output also shows up? You can place compile_fail! to make the compilation fail

Copy link
Author

Choose a reason for hiding this comment

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

I am unsure what you mean. Currently, the function_component is simply a re-export and is not marked as deprecated. If you wish this to be the case, I can change that, I wasn't sure what the preferred behaviour was.

packages/yew-macro/tests/html_macro/generic-component-fail.rs
}

fn main() {}
use std::marker::PhantomData;
Copy link
Member

Choose a reason for hiding this comment

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

What are the changes here? It shows every line as removed and then added

Copy link
Author

Choose a reason for hiding this comment

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

I don't know why this happened. I also saw this after my commit. The same holds for the other file you also mentioned. I do not know what I should do here, because I can't really revert anything as nothing has changed...

Copy link
Author

Choose a reason for hiding this comment

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

After some googling, I think I know why. I originally started working on my desktop and naively copied over some files to my laptop to finish this PR. My desktop uses windows and therefore maybe uses different line feed endings (the \r\n vs \n).

I am new to making contributions, do you know how I can rectify this? Should I make another commit where I take a copy from the original files and replace them?

packages/yew-macro/tests/function_component_attr/generic-lifetime-fail.rs
}

fn main() {}
use yew::prelude::*;
Copy link
Member

Choose a reason for hiding this comment

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

Why is showing diff on unchanged files?

@kirillsemyonkin kirillsemyonkin mentioned this pull request Nov 1, 2023
Merged
2 tasks
@kirillsemyonkin kirillsemyonkin mentioned this pull request Nov 13, 2023
Merged
@kirillsemyonkin kirillsemyonkin mentioned this pull request Dec 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ranile ranile ranile left review comments

@futursolo futursolo futursolo approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
A-yew Area: The main yew crate A-yew-macro Area: The yew-macro crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tdooms @futursolo @ranile