Discussion on: Conditionally render react components in cleaner way

View post

Replies for: For me HOCs in general are an anti-pattern, you can always find better solutions using hooks and context. In this particular case showing the righ...

If your concern is about defaults, you can always use the enum solution with the ?? operator:

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

const Profile = ({ role, ...props }) => {
    const Component = roleComponentMap[role] ?? DefaultProfile;

    return <Component {...props} />;
};

or you can just use default values:

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

const Profile = ({ role = 'default', ...props }) => {
    const Component = roleComponentMap[role];

    return <Component {...props} />;
};

In regards to "readability", and maintainability I could argue that this:

// Declare map
const somethingMap = {
  foo: Something,
  bar: SomethingElse
};
// Then use it
somethingMap[value];

...feels better than this:

// declare function with a switch inside of it
const somethingMap = value => {
  switch (value) {
    case "foo": return Something;
    case "bar": return SomethingElse;
  }
};
// then use it
somethingMap(value);

Victor Vincent • Feb 24 '21

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.

Luke Shiru • Feb 24 '21 • Edited on Feb 24

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} />;
};

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} />;
};

Victor Vincent • Feb 24 '21

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.

Luke Shiru • Feb 24 '21 • Edited on Feb 24

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} />;
};

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} />;
};

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>
    }
}

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.

DEV Community

DEV Community is a community of 850,636 amazing developers

Discussion on: Conditionally render react components in cleaner way

Replies for: For me HOCs in general are an anti-pattern, you can always find better solutions using hooks and context. In this particular case showing the righ...