[Feature Request]: HeaderContainer should either treat its render children as a function or expose a props pass-through

[lluisrojass]

The problem

The UI Shell <HeaderContainer /> component expects render to be a React component. Using the render props pattern (shown in the official storybook example here) will trigger un/re-mounting, which also remounts all children, dumping all state.

An effective solution might be to hoist state above <HeaderContainer />, however this promotes poor organization, and does not handle for all issues (e.g. child img sources are re-fetched causing visual jitter). A user could also provide a React component for render, with the limitation that it be severed from accessing any local parent state or depend on any external prop, which would only leave room for trivial use cases, or require convoluted workarounds.

The solution

We could treat render as a function and not a component, however it would represent a non-backwards compatible change which would require a major version upgrade according to semver.

A simple, backwards compatible solution would be to expose a props pass-through object in HeaderContainer which is spread onto the child component.

Examples

No response

Application/PAL

No response

Business priority

Low Priority = release date is not dependent on fix or not upcoming

Available extra resources

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[github-actions]

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

[tay1orjones]

Using the render props pattern (shown in the official storybook example here) will trigger un/re-mounting, which also remounts all children, dumping all state.

@lluisrojass Hey I'm just a bit confused here, let me see if I can clarify. You're suggesting that any prop changes you provide to the stuff within render is going to cause a rerender - and if we had a prop passthrough it would avoid this due to the shallow prop comparison react does to determine if a re-render is necessary?

Wouldn't a more robust solution be for you to use useMemo or useCallback on what you pass to render?

[lluisrojass]

Nope, sorry maybe I can produce an example in a bit, but re-renders to HeaderContainer will produce an unmount/remount to everything being rendered inside the render prop. That is the core issue.

[lluisrojass]

Hello, here is a repro. You can see the re-mounts with both the log and the image refetching: https://stackblitz.com/edit/github-gab1sd-clhl35?file=src%2FDiceRoll.jsx.

We could define a whole Component to pass into render, but we would loose access to dice state.

Your useMemo/useCallback suggestion would also be subject to re-mounts because we would need to re-generate when dice state changes. Also reasonable heads disagree about whether this is an anti-pattern or not (not saying it is / isn't) (source: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-unstable-nested-components.md#disallow-creating-unstable-components-inside-components-reactno-unstable-nested-components
).

edit: useMemo would work so long as your component did not need any data injected into render e.g. isSideNavExpanded. I still feel the best route would be to fix this internally, but interested to hear your opinion. Actually it still re-mounts.

[tay1orjones]

A simple, backwards compatible solution would be to expose a props pass-through object in HeaderContainer which is spread onto the child component.

Let's try it! 👍 A PR would be awesome if you're interested

[github-actions]

The Carbon team has accepted this proposal! Our team doesn't have the capacity to work on this now, so we are requesting community contributors. Please see the labels for roles that are needed. If you are willing to help out, comment below and we will get in touch!

[lluisrojass]

Ok great, seeing this now, will spend some time on this tomorrow