DEV Community

Discussion on: Conditionally render react components in cleaner way

Collapse
wintercounter profile image
Victor Vincent

Default is not my concern at all, it can be replaced with case 'guest'. My problem is using an enum:

  • with component definitions (foo: <Bar />): it has performance costs
  • with direct component values (foo: Bar): it sacrifices flexibility and maintainability.

The last example you wrote is also an anti-pattern I explained.

As a hint, this code snippet alone here should be a red flag in the 90% of the cases: <Component {...props} />. By seeing the code you don't know what your component exactly is, and what props it is getting. This could be the equivalent of TS's any in React.

Thread Thread
lukeshiru profile image
LUKESHIRU • Edited on

Can you expand on the "cost" of flexibility and maintenance of component values? I mean if the props are the same for every component (which is expected if you're returning any of those out of the same function/component), then there is no issue with it.

As for the ...props, that depends on your setup. It might be a "red flag" in vanilla JS, but it isn't one if you have a type system. Here's an example with TS to illustrate:

import { FC } from "react";

type ProfileProps = JSX.IntrinsicElements["div"];
type ProfileComponent = FC<Props>;

const AdminProfile: ProfileComponent = props => <div {...props} />;
const UserProfile: ProfileComponent = props => <div {...props} />;
const DefaultProfile: ProfileComponent = props => <div {...props} />;

const roleComponentMap = {
  admin: AdminProfile,
  user: UserProfile,
  default: DefaultProfile
};

export const Profile: FC<ProfileProps & {
  readonly role?: keyof typeof roleComponentMap;
}> = ({ role = "default", ...props }) => {
  const Component = roleComponentMap[role]; // Component here is of type ProfileComponent 🎉

  return <Component {...props} />;
};
Enter fullscreen mode Exit fullscreen mode

Obviously this shouldn't be all on the same place, types in their own files, components as well ... but hey, is an example! ... Now, if you want to be extra safe because you don't trust TS, then you can change the Profile component a little:

export const Profile: FC<ProfileProps & {
  readonly role?: keyof typeof roleComponentMap;
}> = ({ role = "default", ...props }) => {
  const Component = roleComponentMap[role] ?? DefaultProfile;

  return <Component {...props} />;
};
Enter fullscreen mode Exit fullscreen mode
Thread Thread
wintercounter profile image
Victor Vincent

That it needs an extra tool (TS) to make it somewhat useable imo already validates my concern. You still cannot SEE from the code what it is.

Why is it so obvious that all components will receive the same prop always? Even in the example there is the logged out case which makes this invalid. Sure, it works fine until I need more/different props, but why would I settle down with a solution that doesn't let me modify later? Especially when the other solution doesn't cost me any extra effort over the enum one.

Thread Thread
lukeshiru profile image
LUKESHIRU • Edited on

Even in the example there is the logged out case which makes this invalid.

I used DefaultProfile instead, but ok...

import { FC } from "react";

type ProfileProps = JSX.IntrinsicElements["div"];
type ProfileComponent = FC<Props>;

const AdminProfile: ProfileComponent = props => <div {...props} />;
const UserProfile: ProfileComponent = props => <div {...props} />;
const LoggedOut: ProfileComponent = props => <div {...props}>Login to see your profile</div>;

const roleComponentMap = {
  admin: AdminProfile,
  user: UserProfile,
  default: LoggedOut
};

export const Profile: FC<ProfileProps & {
  readonly role?: keyof typeof roleComponentMap;
}> = ({ role = "default", ...props }) => {
  const Component = roleComponentMap[role];

  return <Component {...props} />;
};
Enter fullscreen mode Exit fullscreen mode

but why would I settle down with a solution that doesn't let me modify later?

I agree, solutions should take the future into consideration, but what in the code above makes you think you can't modify it later? Let's say now I want to add a new property "color" to Profile, that will set the classname of the rendered profile component to be color-${color}, with the above approach is quite easy:

export const Profile: FC<ProfileProps & {
  readonly role?: keyof typeof roleComponentMap;
  readonly color?: string;
}> = ({ className, color, role = "default", ...props }) => {
  const Component = roleComponentMap[role];

  return <Component className={color ? ` color-${color}` : undefined} {...props} />;
};
Enter fullscreen mode Exit fullscreen mode

With the switch/case, to achieve the same, I have to actually...

const Settings = ({ color }) => {
    const { role } = useContext(SessionContext)
    const className = color ? `color-${color}` : undefined;

    switch (role) {
        case 'admin': return <AdminSettings className={className} />
        case 'user': return <UserSettings className={className} />
        default: return <LoggedOut className={className}>Login to see your profile</LoggedOut>
    }
}
Enter fullscreen mode Exit fullscreen mode

The "initial" cost is pretty much the same, but the long term cost of the switch/case in this scenario is higher. When you're wrapping different components inside a single component (which is the case for this component), is better to share props across them because they will be used in the same places, so ideally they should receive the same props. They might ignore some of them, and use others.