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

Variants derive macro #1616

Merged
merged 30 commits into from
Nov 21, 2020
Merged

Variants derive macro #1616

merged 30 commits into from
Nov 21, 2020

Conversation

K4rakara
Copy link
Contributor

Description

Adds the functionality described in #1613.

#[derive(Clone, Variants)]
enum MyVariants {
  MyFirstComponent(MyFirstComponentProps),
  MySecondComponent(MySecondComponentProps),
}

struct MyItem {
  props: MyVariants,
}

impl<CHILD> From<VChild<CHILD>> for MyItem
where:
  CHILD: Component,
  CHILD::Properties: Into<MyVariants> // <===== THIS IS WHAT THE #[derive(Variants)] MACRO IMPLEMENTS
{
  fn from(vchild: VChild<CHILD>) -> Self {
    MyItem {
      props: vchild.props.into(),
    }
  }
}

impl Into<Html> for MyItem {
  fn into(self) -> Html {
    match self.props {
      MyVariants::MyFirstComponent(props) => VComp::<MyFirstComponent>::new(props, NodeRef::default(), None).into(),
      MyVariants::MySecondComponent(props) => VComp::<MySecondComponent>::new(props, NodeRef::default(), None).into(),
    }
  }
}

Checklist

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

Note: I have not added docs for this feature. I'm not sure if that's a requirement for pull requests though, as its not mentioned in CONTRIBUTING.md.

@siku2 siku2 linked an issue Oct 13, 2020 that may be closed by this pull request
3 tasks
Copy link
Member

@siku2 siku2 left a comment

Choose a reason for hiding this comment

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

I just realised that in its current state, this is really just EnumFromInner from enum_derive, or, for a more modern example, derive_more's From.
Unless we can come up with a way to expand this to be significantly more useful for Yew than these other tools, I don't think we can justify maintaining this.

Instead, we could expand the documentation with a section about how to do this, linking to the available derive macros out there.
What do you say?

I only realised this after finishing the rest of the review and I'm not gonna throw away all this work 😅

yew-macro/src/lib.rs Outdated Show resolved Hide resolved
yew-macro/tests/derive_variants/fail.rs Outdated Show resolved Hide resolved
yew-macro/src/derive_variants/mod.rs Outdated Show resolved Hide resolved
yew-macro/src/derive_variants/mod.rs Outdated Show resolved Hide resolved
yew-macro/src/derive_variants/mod.rs Outdated Show resolved Hide resolved
yew-macro/src/derive_variants/variant.rs Outdated Show resolved Hide resolved
yew-macro/src/derive_variants/variant.rs Outdated Show resolved Hide resolved
yew-macro/tests/derive_variants_test.rs Outdated Show resolved Hide resolved
yew-macro/tests/derive_variants/pass.rs Outdated Show resolved Hide resolved
@siku2 siku2 added macro Issues relating to our procedural or declarative macros ergonomics labels Oct 13, 2020
@K4rakara
Copy link
Contributor Author

That sounds fine too! I was completely unaware of that crates existence. Are the docs generated inline like standard Rust docs, or are they done in some other way?

@siku2
Copy link
Member

siku2 commented Oct 13, 2020

The Yew Docs? They're generated by Docusaurus which you can find in the website directory (that's also where you start the thing). The actual content is located in the docs directory though.

Typed children are mentioned here: docs/concepts/html/components.md
We could add a link there to a more elaborate description which mentions the enum pattern. That should go to the docs/concepts/components group.

Also pinging @teymour-aldridge for input.

@K4rakara
Copy link
Contributor Author

I'll see if I can put something together 😄

@github-actions
Copy link

github-actions bot commented Oct 18, 2020

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

https://yew-rs--pr1616-variants-derive-macr-falhdmlv.web.app

(expires Sat, 28 Nov 2020 01:27:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@siku2 siku2 added documentation and removed ergonomics macro Issues relating to our procedural or declarative macros labels Oct 18, 2020
@siku2
Copy link
Member

siku2 commented Oct 18, 2020

Please update website/sitebars.json for the new page to show up.

Copy link
Contributor

@teymour-aldridge teymour-aldridge left a comment

Choose a reason for hiding this comment

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

Sorry for taking a while to get to this.

Note: these are all just suggestions (except a few typo fixes), feel free to voice disagreement 🙃

docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
Comment on lines 44 to 45
- You want to restrict the types of components that can be used as children
on this component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- You want to restrict the types of components that can be used as children
on this component.
- You want to only allow objects of a specific type to be used as children for your component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of this I agree with, but is objects the right term here?

docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
website/sidebars.json Outdated Show resolved Hide resolved
K4rakara and others added 6 commits October 21, 2020 22:14
on -> of

Co-authored-by: Teymour Aldridge <[email protected]>
Typo fix

Co-authored-by: Teymour Aldridge <[email protected]>
Reword

Co-authored-by: Teymour Aldridge <[email protected]>
Something else => an alternative method

Co-authored-by: Teymour Aldridge <[email protected]>
Rewording

Co-authored-by: Teymour Aldridge <[email protected]>
Capitialization fix

Co-authored-by: Teymour Aldridge <[email protected]>
docs/concepts/components/properties.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Outdated Show resolved Hide resolved
docs/concepts/components/children.md Show resolved Hide resolved
Copy link
Member

@siku2 siku2 left a comment

Choose a reason for hiding this comment

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

Sorry, I expected to get this done much sooner but didn't find the time.
I promised to suggest an improved example for the "Sharing properties" section but I ran into some problems. I've added a comment there to explain it.

docs/concepts/components/children.md Outdated Show resolved Hide resolved
@siku2 siku2 merged commit 9ac6b05 into yewstack:master Nov 21, 2020
@siku2
Copy link
Member

siku2 commented Nov 21, 2020

It's great that we finally have some documentation for children 🎉
Thanks a lot for bearing with me :)

@siku2 siku2 added this to the v0.18 milestone Nov 21, 2020
@K4rakara
Copy link
Contributor Author

Looking forward to contributing more to Yew in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variant derive macro
4 participants