How can I re-write the conditionals for this button

Question

I've been trying out React for a couple of days and I tried to build a button component with some conditionals. Everything works but I feel like I am not doing it the right way.

Could anyone review my code and give me some advice and tell me what I am doing wrong or what i could do better. I think I am using too many return statements.

I use Typescript for type checking.

Would appreciate any help!

import { Link } from "gatsby";

interface Props {
  content: string,
  color: string,
  path?: string,
  type?: string
}

export default function Button(props: Props): JSX.Element {
  let elementType;

  const checkProps = () => {
    const availableColors = ['blue', 'grey', 'dark-grey', 'white'];
    const borderColors = [''];
    let buttonStyle;

    // Check if props.color exists in availableColors
    if(!availableColors.includes(props.color)) {
      console.error(`"${props.color} color key not available for button style`);
      return;
    }
    
    buttonStyle = buttonStyles[`button--${props.color}`];

    // Check if props.content is valid
    if(!props.content) {
      console.error(`"${props.content} is not valid as content`);
      return;
    }

    // Check if props.type exists
    if(!props.type) {
      elementType = <button className={ buttonStyle }>{ props.content }</button>
      return;
    }

    if(props.type !== 'link') {
      console.error(`"${props.type} is not a valid type, use 'link' instead or use nothing for default button`);
      return;
    } else {
        elementType = <Link to={ props.path } className={ buttonStyle }>{ props.content }</Link>
    }
  }

  checkProps();

  return (
    <React.Fragment>
      { elementType }
    </React.Fragment>
  );
}```

Answer 1

Narrow types with TypeScript instead of erroring There are many places in the code where you check a condition based on how the Button is being called, and if it isn't fulfilled, you log an error and return nothing. This is a reasonable approach in JavaScript, but in TypeScript, you have a much better option, which is to use types to require only certain arguments. This way, rather than an error being thrown at runtime, an error will be thrown by TypeScript at compile-time when a user of Button attempts to pass props that aren't valid. Turning typos / accidental incorrect function calls into compile-time errors is one of the greatest advantages of TypeScript, and it's great that you're using TypeScript already, so go ahead and take advantage of it.

For example, rather than:

const availableColors = ['blue', 'grey', 'dark-grey', 'white'];
const borderColors = [''];
let buttonStyle;

// Check if props.color exists in availableColors
if(!availableColors.includes(props.color)) {
  console.error(`"${props.color} color key not available for button style`);
  return;
}

You can make availableColors as a type instead, and require that the color prop matches it:

'blue' | 'grey' | 'dark-grey' | 'white'

Similarly, rather than

if(!props.content) {
  console.error(`"${props.content} is not valid as content`);

you can require that the content property is of type:

T & (T extends '' ? 'Content must not be empty' : {})

and instead of

if(props.type !== 'link') {

require a type of 'link' | '' exactly.

Using the above methods will also let you declare buttonStyle and elementType with const - or you could even inline their values into the props.

Default export? You have:

export default function Button

This will mean that the imported button is not really tied to its Button name anymore. For example, it would be easy for someone to initially type:

import button from './Button';

only to run into problems since they forgot to capitalize it. Using named exports makes these sorts of typos more difficult, because one has to intentionally rename it to get something other than Button:

// Natural import
import { Button } from './Button';

or

// Deliberate renaming
import { Button as RenamedVariableName } from './Button';

Destructure immediately Rather than referencing props multiple times in the function body, you might consider extracting the properties from the props immediately in the function definition - it makes the logic later less noisy.

Let TS infer types when possible There's no need to note that a function returns a particular type if

• TypeScript can already infer the type automatically, and
• The type is obvious to a reader of a code at a glance

So, you may consider removing the JSX.Element from (props: Props): JSX.Element.

buttonStyles? It sounds like buttonStyles is an object that contains strings, where the strings are class names. So buttonStyles isn't a very precise variable name - it doesn't contain styles, it contains class names. Maybe instead call it buttonClassNamesByColor?

Fragments In newer versions of React, you may use <> and </> instead of <React.Fragment> </React.Fragment>. Or, even better, since you're returning only a single JSX element regardless, you can use the conditional operator to return either one or the other element, without wrapping everything in a fragment.

export const Button = <T extends string>({
    content,
    color,
    path,
    type,
}: {
    content: T & (T extends '' ? 'Content must not be empty' : {});
    color: 'blue' | 'grey' | 'dark-grey' | 'white';
    path?: string;
    type: '' | 'link';
}) => {
    const buttonClassName = buttonClassNamesByColor[`button--${color}`];
    return type === ''
        ? <button className={buttonClassName}>{content}</button>
        : <Link to={path} className={buttonClassName}>{content}</Link>;
}