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

Allow function_component creation based on function name #2292

Merged
merged 5 commits into from Dec 23, 2021
Merged

Allow function_component creation based on function name #2292

merged 5 commits into from Dec 23, 2021

Conversation

hamza1311
Copy link
Member

@hamza1311 hamza1311 commented Dec 22, 2021 •
edited

Description

This PR modifies the function_component macro to use the function name if no name is provided.

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests
  • I have added docs

@github-actions
Copy link

github-actions bot commented Dec 22, 2021 •
edited

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

https://yew-rs--pr2292-enhance-fc-r0osy4ih.web.app

(expires Thu, 30 Dec 2021 12:12:25 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@voidpumpkin voidpumpkin added A-yew Area: The main yew crate A-yew-macro Area: The yew-macro crate labels Dec 22, 2021
voidpumpkin
voidpumpkin previously approved these changes Dec 22, 2021
View reviewed changes
Copy link
Member

@voidpumpkin voidpumpkin left a comment

Choose a reason for hiding this comment

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

Really good thanks!

@hamza1311 hamza1311 requested a review from siku2 December 22, 2021 19:59
@hamza1311 hamza1311 enabled auto-merge (squash) December 22, 2021 19:59
@futursolo
Copy link
Member

futursolo commented Dec 22, 2021 •
edited

Maybe pub comp Component() -> Html instead of pub fn Component -> Html?

Since the internal was never a function and this kind of justifies the non snake naming scheme of component names.

I would prefer functions to have snake_case if it was marked as pub fn.

@Madoshakalaka
Copy link
Contributor

Cool Stuff!

Madoshakalaka
Madoshakalaka reviewed Dec 23, 2021
View reviewed changes
packages/yew-macro/src/function_component.rs
Comment on lines +178 to +183
let component_name = component_name.unwrap_or_else(|| function_name.clone());
let function_name = format_ident!(
"{}FunctionProvider",
function_name,
span = function_name.span()
);
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to convert the function name to PascalCase with convert_case?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't really matter since it's not in the public API. Besides, generally this name would be PascalCase anyway because that's the convention for component names.

@hamza1311
Copy link
Member Author

Maybe pub comp Component() -> Html instead of pub fn Component -> Html?

Unfortunately, that's not possible. rustc will reject it since comp can't be as such. The input to attribute macros has to be valid Rust code.

@hamza1311 hamza1311 merged commit 73b5f6b into yewstack:master Dec 23, 2021
@hamza1311 hamza1311 mentioned this pull request Dec 23, 2021
Merged
3 tasks
da-x pushed a commit to da-x/yew that referenced this pull request Jul 27, 2022
@hamza1311 @da-x
Allow function_component creation based on function name (yewstack#…
55c98f2 
…2292)

* function_component without name

* fmt

* fc

* update docs

* remove fc
da-x pushed a commit to da-x/yew that referenced this pull request Jul 27, 2022
@hamza1311 @da-x
Allow function_component creation based on function name (yewstack#…
b49c6b7 
…2292)

* function_component without name

* fmt

* fc

* update docs

* remove fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Madoshakalaka Madoshakalaka Madoshakalaka left review comments

@siku2 siku2 siku2 approved these changes

@voidpumpkin voidpumpkin voidpumpkin approved these changes

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
5 participants
@hamza1311 @futursolo @Madoshakalaka @siku2 @voidpumpkin