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

Better Function Component in docs + Macro-based Hooks #2478

Merged
merged 18 commits into from
Mar 8, 2022
Merged

Better Function Component in docs + Macro-based Hooks #2478

merged 18 commits into from
Mar 8, 2022

Conversation

futursolo
Copy link
Member

@futursolo futursolo commented Feb 27, 2022 •
edited by ranile
Loading

Description

Fixes #2144
Closes #2435 (supersedes it)

  1. Function components are no longer a type definition.
  2. Implements macro-based hooks like: use_style!(r"color: red;")
  3. #[allow] and #[doc] are now copied to outer struct.
  4. function components with const generics are restored.
  5. Fix SealedBaseComponent.
  6. AttrValue is now prelude.

image

Checklist

@github-actions
Copy link

github-actions bot commented Feb 27, 2022 •
edited
Loading

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

https://yew-rs-api--pr2478-fn-comp-2-part-2-mhkhgqv9.web.app

(expires Sun, 13 Mar 2022 05:09:58 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@futursolo futursolo changed the title Macro-based Hooks Better Function Component in docs + Macro-based Hooks Feb 27, 2022
@ranile
Copy link
Member

ranile commented Mar 6, 2022

@futursolo can you update this PR now that #2330 has been merged?

@futursolo
Merge commit '78d4204a9a52a826bd2be4e7383d29e04233fb5b' into fn-comp-…
db3324d 
…2-part-2

# Conflicts:
#	packages/yew/src/dom_bundle/app_handle.rs
#	packages/yew/src/html/component/scope.rs
#	packages/yew/src/lib.rs
#	packages/yew/src/server_renderer.rs
#	packages/yew/src/virtual_dom/listeners.rs
#	packages/yew/src/virtual_dom/vcomp.rs
#	packages/yew/src/virtual_dom/vnode.rs
#	tools/process-benchmark-results/src/main.rs
@futursolo futursolo mentioned this pull request Mar 6, 2022
Merged
3 tasks
@voidpumpkin voidpumpkin added the A-yew-macro Area: The yew-macro crate label Mar 7, 2022
voidpumpkin
voidpumpkin approved these changes Mar 7, 2022
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.

Looks good to me.

@voidpumpkin voidpumpkin added the A-yew Area: The main yew crate label Mar 7, 2022
ranile
ranile reviewed Mar 7, 2022
View reviewed changes
Copy link
Member

@ranile ranile left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just a couple of comments. Not blocking but I would like to have them fixed before merging.

One question though: Was BaseComponent not enough that we need to introduce SealedBaseComponent and IntoComponent? What was wrong with BaseComponent?

PS: I updated the PR description to say "Closes #issue" so the PR is closed on merge

packages/yew-macro/src/function_component.rs
m.path
.get_ident()
.and_then(|ident| match ident.to_string().as_str() {
"doc" | "allow" => Some(m.clone()),
Copy link
Member

Choose a reason for hiding this comment

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

We should pass deny and warn. if we're passing along allow, or does it not matter?

Copy link
Member Author

Choose a reason for hiding this comment

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

We probably shouldn't. This makes users to be able to apply lints on generated code.

Lints are usually suppressed on generated code.

packages/yew-macro/src/function_component.rs Show resolved Hide resolved
packages/yew-macro/src/hook/mod.rs Show resolved Hide resolved
@futursolo
Copy link
Member Author

futursolo commented Mar 7, 2022 •
edited
Loading

One question though: Was BaseComponent not enough that we need to introduce SealedBaseComponent and IntoComponent? What was wrong with BaseComponent?

IntoComponent makes it possible to not copy the entire implementation of FunctionComponent<T> into the procedural macro.

Sealing BaseComponent makes it possible to implement frequent incompatible changes solely for function component without breaking any user implemented components.
(For example, Yew's use_effect is not equivalent to React's useEffect and updating it requires an incompatible change to BaseComponent, which does not make sense for struct components.)

In addition, I think BaseComponent no longer holds some of the guarantees that Component has.
(You can receive messages before rendered is called, etc.)

ranile
ranile approved these changes Mar 8, 2022
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.

LGTM

@ranile ranile merged commit 51238fb into yewstack:master Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ranile ranile ranile 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.

Function components look weird in rustdoc
3 participants
@futursolo @ranile @voidpumpkin