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

Document anti-pattrens regarding props #1843

Closed
Tracked by #2312 ...
ranile opened this issue May 8, 2021 · 5 comments · Fixed by #2817
Closed
Tracked by #2312 ...

Document anti-pattrens regarding props #1843

ranile opened this issue May 8, 2021 · 5 comments · Fixed by #2817
Labels
documentation good first issue meta Repository chores and tasks
Milestone

Comments

@ranile
Copy link
Member

ranile commented May 8, 2021

I think we should document anti-patterns. I would be fine with making a PR for those but first we need to list them.

  1. Taking String, instead of Cow<'static, str> as props.
  2. What else?

From #1834 (comment), as requested by @siku2

@siku2 siku2 added documentation meta Repository chores and tasks labels May 8, 2021
@ranile ranile changed the title Tracking issue for documenting anti-pattrens regarding props Document anti-pattrens regarding props May 17, 2021
@martinitus
Copy link

martinitus commented Sep 13, 2021

For a beginner (like me) who also doesn't have any significant frontend / react experience some more general "dos & and don'ts" would also be really helpful. Questions I am uncertain about include stuff like:

  • How much data should/could I pack into props: like 10000 structures that get rendered as a list/child elements?
  • How do I enable a component to fetch data (e.g. via an agent)? Should the handle to the agent be part of the props or of the struct?
  • When should I put stuff into the props, and which kind of objects should I maintain in the actual component struct?
    Hth

@ranile ranile added this to the v0.20 milestone Dec 28, 2021
@ranile
Copy link
Member Author

ranile commented Jan 1, 2022

A section about anti-patterns/what not to do on the Properties page would be enough.

@finnbear
Copy link
Contributor

In #2402, it seems like the consensus was to use some form of Rc<str> instead of String/Cow<'static, str> (which clone String's too often)

It seems to me that the yew::virtual_dom::AttrValue type does the job well, and the ergonomics (implicit conversions) seem to be in place. The only issue I see is that this type is exported by the virtual_dom module, which implies it isn't intended for general use as a property type.

Is using AttrValue for component properties (that the end-user may supply a static or non-static string for) worthy of mention in the new docs, or is it also an anti-pattern, and if so, what is the better alternative?

@WorldSEnder
Copy link
Member

WorldSEnder commented Jun 10, 2022

@finnbear AttrValue is the type of properties that eventually end up as attributes of html elements. That is the intended application, although in its current form, it serves quite well as a general Rc<str>.

Using it generally for attributes of type Rc<str> leaves a bit of a sour taste. A better alternative is proposed in #2563, i.e. implicit_clone::unsync::IString - I have to catch up on reviewing that again. AttrValue might do extra work, since the string will likely end up in javascript land, and a lot of attributes have a fixed set of values: It might make sense in the future to intern the value during reconciliation.

@cecton
Copy link
Member

cecton commented Jun 16, 2022

@WorldSEnder I'm pretty sure AttrValue is almost 100% identical to IString. When I made it I ended up with almost the exact same code. The only difference back then is that AttrValue had a variant for String which I removed in #2447 because it wasn't useful and less performant than Rc<str>.

My recommendations on props:

  • It's important for performance reasons to avoid cloning because the detection of changes is called very often. That's the whole point of Introduce immutable string, array and map #2563.
  • Everything that you put in Props should be cheap to clone (repeating myself sorry)... (:thinking: come to think of it... maybe this could be enforced the same way that Copy enforce all the fields of a struct to be Copy?)
  • Larger structures should be wrapped in Rc (so they become cheap to clone)
  • Props initialization (when you create the value) of big structure should be placed in the fn create() method and you should avoid to create them in fn view() method. Especially things that allocates (Vec, HashMap, etc...)
  • Callbacks should be created in the fn create() method too: this is because if you create them in view() and pass them as prop, it will be identified as a different value every time the component checks for update (got bitten by that recently)

Unfortunately I haven't work much with functional component yet (I did some react hooks for a client a year ago though) but I think some of the issues with full components are better handled with the hook mechanism. Maybe someone more experimented with functional components in Yew can detail this and compare with the list of recommendations I just did for full components?

One last recommendation I have in a more general way: if you worked with React you will be tempted to solve problems the way you usually did with React. Sometimes those solutions don't work at all with Rust and you need to take a step back and think more of the Rust way. For example, in JS we use objects (hashmaps) a lot but in Rust it often makes little sense. (I, for one, almost never use hashmaps.)

(Main reason I didn't hook with the hook is because I already had a big code base and lots of habits 😓)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation good first issue meta Repository chores and tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants