Conditional Rendering and Children component interact strangely #3256

sinistersnare · 2023-05-03T20:59:18Z

Problem
I am trying to conditionally render some HTML that goes through a Children component. When using enumerate() on the Children iterator, should-be-skipped components are rendered anyways.

Steps To Reproduce
Steps to reproduce the behavior:

Go to https://play.yew.rs/?shared=VQIs2qSmnXjGJl8OYX5s and run it.
Notice that C is not rendered, but is still given an iteration.

Expected behavior
The enumerator should only give 3 iterations, skipping the would-be C iteration.

I assume that the x iterator is getting some ghost value at some point. Can I just check for that instead as a workaround? What would that look like?

Environment:

Whatever is on Yew playground on May 3, 2023

Browser and version, Firefox 112, did not try on others.

Questionnaire

I'm interested in fixing this myself but don't know where to start
I would like to fix and I have a solution
I don't have time to fix this right now, but maybe later

The text was updated successfully, but these errors were encountered:

WorldSEnder · 2023-05-05T08:02:29Z

The "omitted" element is rendered as an empty fragment, <></>, if the condition evaluates to false. I suppose this could be change to simply omit it from the children list instead.

sinistersnare · 2023-05-05T13:30:45Z

I solved this by just moving the condition outside of the html! macro call. This means more code-duplication but overall thats fine. It may be a more difficult problem when dealing with more complex conditoons, but for me I have worked around it.

If this is considered a bug, feel free to keep it open!

texodus · 2023-05-21T15:25:18Z

I opened a PR to change this but was convinced by @futursolo that omitting these is not the semantically correct fix.

The problem remains though. The insertion of a default else branch for the conditionals is inconsistent, in that no other unit-typed expression is allowed by Yew:

// not allowed
html! {
    { () }
}

I understand if this is considered pedantic, there's IMO no need to fix this if it is community-disruptive. If there is appetite to fix it, I propose either:

Add an Into<Html> implementation for () which returns an empty template. This allows various syntax to be rendered as an inert template, which in current Yew is an error, so maybe undesirable as a an API design choice.
Disallow conditionals without an else branch, as is currently done for all other unit-typed expressions.

futursolo · 2023-05-22T10:05:31Z

in that no other unit-typed expression is allowed by Yew:

When you write { () }, you have explicitly passed a unit type as the return type. Similarly, if b { () } isn't allowed either.

An argument can be raised for {} if it is allowed as {} has no return type which should use the default return type.
But html! disallows an empty block expression just like if b {}, so we avoided the default value type for blocks that is declared.

For the issue itself, I think there is a discrepancy in the design of Children and keys:

Yew allows inspection / enumeration of Children / ChildrenRenderer<...>.
Keyless-safe conditional rendering are implemented with an important but subtle implementation detail.
Processing Children item by removing / adding items can lead to surprising behaviour unless fully keyed.
For callers of a component, it is hard to predict whether such manipulation is done so it is difficult to determine whether keys are required for such cases. It's also not practical to tell users to key every item passed as Children for every component.

Ideally, like in React, if you wish to further process children, you need to pass it in a way other than React.ReactNode, usually as an object or array. ReactNode is usually not meant to be manipulated but used as-is.

This provides 2 advantages:

Key requirements are clear. That the parent component can force the data structure passed to it to be fully keyed.
You can define an implementation that omits the child component / element reliably and correctly, given keys are properly provided.

I am actually fine with making Children opaque hence removing inspection API so people can opt for a React-like approach on children after #3042, but I guess this is a bigger change and might not be well received by the community.

WorldSEnder · 2023-05-22T16:47:25Z

I am actually fine with making Children opaque hence removing inspection API so people can opt for a React-like approach on children after #3042, but I guess this is a bigger change and might not be well received by the community.

This will most likely break quite a few existing components, especially those that are used in non-trivial applications, which makes it even harder to find work-arounds and alternatives for those if we were to do this. VDom should stay introspectable, but on the other hand I think breaking changes are to be expected and implied if one ends up using that introspection API.

Can we have both? For example, with the following - maybe too subtle, but I think teachable - semantic change. Currently there are a few syntaxes with similar, but imo inconsistent, meaning:

html! {
  <Foo>
    // For
    { for vec_of_children.into_iter() } // (Vec1)
    // for vec_of_children.into_iter() // (Vec2) // Not! allowed
    { vec_of_children } // (Vec3)
    // If
    //{ if condition { {element} } } // (If1) // Not allowed!
    if condition { {element} } // (If2)
    { condition.then_some(element) } // (If3)
  </Foo>
}

The for <iterable> construct is recognized inside braces (Vec1), but not allowed without those (Vec2). An expression of type Vec<_> extends the children, and does not start a sub-fragment as is the case in react (Vec3).

The if <conditon> construct is recognized outside of braces (If2), but treated as a rust if expression inside of these (If1). An expression of type Option<_> also extends the children (If3).

Small note: This is also related to #3262. I think it's important that extending the children, instead of the react behaviour of starting sublists, remains possible. Think of a list where I want to separate elements into two categories, e.g. "inactive" and "active" and render <li><Header />{active}<Header />{inactive}</li>. I want elements to be retained when switching from active to inactive. For this, all elements should be part of the same list. Keys work in yew, but not with the behaviour of react.

I would expect that all six cases here work! But, with slight changes in semantics. Using braces - cases (Vec1, Vec3, If1, If3) - should lead to exactly one child in the parent list, i.e. a nested fragment which is then reconciled as a list of its own like in react. Vec2 and If2 should be useable for a variable number of children. Vec2 extends the list with those elements from the iterator. If2 extends the list if the condition is true, otherwise inserts no element (instead of the current placeholder) - related to #3157, the proposed workaround there would then be automatic.

futursolo · 2023-05-31T14:21:18Z

React's documentation says the following:

Manipulating children with the Children methods often leads to fragile code. When you pass children to a component in JSX, you don’t usually expect the component to manipulate or transform the individual children.

When you can, try to avoid using the Children methods. ...
https://react.dev/reference/react/Children

React considered their Children API as a Legacy API for the reasons they described in their documentation, which applies to our Children API as well. I think we should consider eventually removing Children as Rust crates should have a higher bar against unsound API. React's Children manipulation API is more limited than ours, but they still cannot eliminated all edge cases. Our Children API allows all iterator methods, which created more unfixable edge cases and enabled users to easier "abusing" the API.

This will most likely break quite a few existing components, especially those that are used in non-trivial applications, which makes it even harder to find work-arounds and alternatives for those if we were to do this. VDom should stay introspectable, but on the other hand I think breaking changes are to be expected and implied if one ends up using that introspection API.

We don't have to make this a breaking change. We can deprecate Children and ChildrenRenderer and ask users to use Html if they don't need manipulation and define a structure that require keys if they do require manipulation should define a fully-keyed structure. Html is also much faster because it doesn't require clone.

We also already broke every non-trivial function component in the next release by changing the dependency order so I think it's fine to have another big change even if it is a breaking change (which it can be done without being a breaking change).

I am also fine with keeping a Children-like with manipulation, as long as we have a way to clearly ensure that:

If a Children isn't keyed, then it is not meant to be manipulated.
If a Children is meant to be manipulated, then it requires to be fully-keyed, preferably at compile time, controllable by the callee (Parent Component). And we have a way to prevent key collision.

However, I still wouldn't recommend this to anyone if we have to explain things like #3256.

I wish to save the discussion that extends for iterator behaviour elements in #3262, so I wouldn't discuss it here. I will focus on why trying to understand what Children contains can be hard and manipulating them can lead to undesirable behaviour here, which is the root cause of this issue.

Why I think Children trying to inspect / manipulate what `html!` Children contains is hard and can lead to undesirable behaviour?

Children allows manipulation that shouldn't be allowed in a seemingly "correct" and "canonical" way.

Children implements IntoIterator. This may give users an impression that all manipulation that is permitted by iterator should be OK.

This means a structure as simple as the following can result in surprising behaviour depending on what Parent is doing to its children. (In fact, every component with Children should be able to produce unintentional behaviour if they wish to.)

<Parent>
    <Child />
    <Child />
    <Child />
</Parent>

Consider this example: https://play.yew.rs/?shared=L8SO6Ob40kZg5coocLLb

For the following component:

<ItemReducer pick_every={*pick_every}>
    <Child order=1 />
    <Child order=2 />
    <Child order=3 />
    <Child order=4 />
    <Child order=5 />
    <Child order=6 />
</ItemReducer>

The ItemReducer component picks out child components that in its dom order, represented in number, is dividable by the pick_every property.

Initially, pick_every is set to 1. Which shows every child.

The Render No part is to see which component is re-rendered and which ones are re-used here.

If you click on the Pick every other child button , pick_every will be set to 2. ItemReducer now tries to pick child components in with an even order number.

This means that the component 2, 4, 6 with Render No 5, 3 and 1 should remain.

However, the result is Render No 3, 2 and 1 remained and other components unmounted. The component states are also persisted for those components not the ones we expected.

This seems very natural to create ItemReducer in this way and assume that Children would behave as what I expected if Children is designed to be manipulated with iterator and I think it's difficult to explain ItemMapper (in the above playground) is OK but ItemReducer isn't when Children.into_iter() is allowed.

It is also very unintuitive for callers (users of ItemReducer) in this case to understand that they need keys for all Children even if they don't even change at all. (Callee should have a better idea, but still very an easy mistake to make.)

What you get with html! isn't always you would assume.

The html macro have multiple ways to represent the same virtual dom structure that itself sometimes considers as "identical". (Such as this issue) There are multiple reasons why this is done. Sometimes due to it makes users life easier, other times it's because of structural integrity.

These should be very trivial things that should be hidden from the end user and requiring them to understand it would be unnecessary burden. Like Rust users should not be required to understand how MIR works, Yew users shouldn't be required to understand how html! works as well.

sinistersnare added the bug label May 3, 2023

texodus mentioned this issue May 19, 2023

Omit conditional elements without an else branch when present in children #3274

Closed

2 tasks

futursolo mentioned this issue Jun 1, 2023

Regression: elements are re-rendered even if they didn't change, when there's a list below them (worked in v0.19.3) #3262

Open

3 tasks

futursolo mentioned this issue Jun 11, 2023

Update documentation around Children #3297

Merged

2 tasks

hamza1311 mentioned this issue Aug 5, 2023

conditional 'if' doesn't work with ChildrenRenderer<T> #3157

Closed

3 tasks

ctron mentioned this issue Dec 13, 2023

docs: explain a workaround for if with typed children #3560

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conditional Rendering and Children component interact strangely #3256

Conditional Rendering and Children component interact strangely #3256

sinistersnare commented May 3, 2023 •

edited

WorldSEnder commented May 5, 2023

sinistersnare commented May 5, 2023

texodus commented May 21, 2023

futursolo commented May 22, 2023

WorldSEnder commented May 22, 2023 •

edited

futursolo commented May 31, 2023 •

edited

Conditional Rendering and Children component interact strangely #3256

Conditional Rendering and Children component interact strangely #3256

Comments

sinistersnare commented May 3, 2023 • edited

WorldSEnder commented May 5, 2023

sinistersnare commented May 5, 2023

texodus commented May 21, 2023

futursolo commented May 22, 2023

WorldSEnder commented May 22, 2023 • edited

futursolo commented May 31, 2023 • edited

Why I think Children trying to inspect / manipulate what html! Children contains is hard and can lead to undesirable behaviour?

sinistersnare commented May 3, 2023 •

edited

WorldSEnder commented May 22, 2023 •

edited

futursolo commented May 31, 2023 •

edited

Why I think Children trying to inspect / manipulate what `html!` Children contains is hard and can lead to undesirable behaviour?