React component which uses another component for rendering

Question

Having this React component:

import RadioButton from './radio-button';
import { Label, Horizontal, Vertical } from './radio-button.styles';
import { RadioOption, OptionGroup } from './radio-button.types';

const RenderedOptions = ({ label, options, onChange }: OptionGroup) => {
  return (
    <>
      {options.map(({ value, name, disabled }: RadioOption, index) => {
        const optionLabel = label.replace(/ +/g, '-');
        const optionId = `${optionLabel}-${index}`;
        return (
          <RadioButton
            value={value}
            label={value}
            key={optionId}
            id={optionId}
            name={name}
            disabled={disabled}
            defaultChecked={index === 0}
            onChange={onChange}
          />
        );
      })}
    </>
  );
};

export const RadioButtonGroup = ({ label, options, horizontal, onChange }: OptionGroup) => {
  const renderedOptions = horizontal ? (
    <Horizontal>
      <RenderedOptions label={label} options={options} onChange={onChange} />
    </Horizontal>
  ) : (
    <Vertical>
      <RenderedOptions label={label} options={options} onChange={onChange} />
    </Vertical>
  );

  return (
    <>
      <Label>{label}</Label>
      {renderedOptions}
    </>
  );
};
export default RadioButtonGroup;

As the name says, it's a reusable Radio Group component which works fine but I have the feeling that it could be done better/cleaner.

Any suggestions?

Answer 1

You can clean up RadioButtonGroup by defining what the layout component should be as a variable

export const RadioButtonGroup = ({ label, options, horizontal, onChange }: OptionGroup) => {
  const Direction = horizontal ? Horizontal : Vertical;
  return (
    <>
      <Label>{label}</Label>
      <Direction>
        <RenderedOptions label={label} options={options} onChange={onChange} />
      </Direction>
    </>
  );
};

Answer 2

In RenderedOptions:

• You can pull const optionLabel = label.replace(/ +/g, '-'); out of the loop to the top of the function since it doesn't need to be recalculated every loop iteration-- it's constant for a given set of options.
• There are good arguments not to use the index as the key (or part of the key). It's an efficiency issue that probably won't matter much here, but some people will flag this as a no-no. The key just needs to be unique within the list, so value should work, if I'm reading things right.

In RadioButtonGroup:

• You could consider using "rest" parameters, since some of the properties are pass-throughs. ie.

export const RadioButtonGroup = ({ horizontal, ...opts }: OptionGroup) => {
  const Direction = horizontal ? Horizontal : Vertical;
  return (
    <>
      <Label>{opts.label}</Label>
      <Direction>
        <RenderedOptions ...opts />
      </Direction>
    </>
  );
};

Hope this helps!