Refactoring

Lukasz Ostrowski

www.ostrowski.ninja

Components

React

Motivation

Project with triple inheritance of React class components

🧐

What is refactoring?

Martin Fowler

Refactoring (noun): a change made to the internal structure of software to make it easier to understand and cheaper to modify without changing its observable behavior. 

🤔

When to refactor?

3️⃣ The third time you do something similar, you refactor. 

2️⃣ The second time you do something similar, you wince at the duplication, but you do the duplicate thing anyway.

 

1️⃣ The first time you do something, you just do it.

The Rule of Three

👃

Code smells scream to be refactored

👃

Code smells

👃

Code smell is any piece in the source code that possibly indicates a deeper problem.

(Dead code, long classes, global variables etc)

Why code starts to be dirty?

👶 Inexperienced developers

🚽 Tech debt (strategic or not)

📋 Catching up with business requirements

🤔 Not enough information about domain

🌱 It just happens when code grows. And it's normal.

✔️ ➡️ ❌ ➡️ ✔️

Test before refactor

You promised React dude

React components are just functions

                 , so apply
functions' best practices

One more thing...

1️⃣

Keep logic separated from the view

const LoginForm = () => {
    const handleSubmit = event => {
        const values = getValuesFromEvent(event); // Call some util

        localStorage.setItem('form', values);
        fetch('/api', { body: JSON.stringify(values) }).then(
            () => {
                // Handle success
            },
        ).catch(err => {
            // Handle error
        });

        // And some other domain/state/business logic
    };

    return (
        <form onSubmit={handleSubmit}>
            <input type="email"/>
            <input type="password"/>
            <button type="submit">Log in</button>
        </form>
    );
};

❌ Don't!

Don't mix business logic with UI

type Props = {
    onSubmit(values: { email: string; password: string }): unknown;
};

const LoginForm = ({onSubmit}: Props) => {
    const handleSubmit = event => {
        const values = getValuesFromEvent(event); // Call some util

        onSubmit(values)
    };

    return (
        <form onSubmit={handleSubmit}>
            <input type="email"/>
            <input type="password"/>
            <button type="submit">Log in</button>
        </form>
    );
};

✅ Try callback instead

Provide data and emit only callbacks / events.

const ComposedLoginForm = compose(
    withLocalStorage,
    withSavingToApi
)(LoginForm)

✅ or compose with HOCs

With HOC each piece of logic can be extracted to separate function

2️⃣

Extract state to separate layer

const CollapseSection = ({ headline, content }) => {
    const [open, setOpen] = useState(false);

    return (
        <div>
            <button onClick={
                () => setOpen(open => !open)}>
                {headline}
            </button>
            {open && <div>{content}</div>}
        </div>
    );
};

❌ Instead of this

This will not work if you need to control state from parent or Redux.

type CollapseProps = {
    headline: ReactNode;
    content: ReactNode;
};

type ToggleProps = {
    open: boolean;
    onToggle(): unknown;
};

const CollapseSection = ({
      headline,
      content,
      open,
      onToggle,
  }: ToggleProps & CollapseProps) => (
    <div>
        <button onClick={onToggle}>{headline}</button>
        {open && <div>{content}</div>}
    </div>
);

✅ Try extracting state

const withToggleState = <Props extends ToggleProps>(
    Component: ComponentType<Props>,
) => (props: Props) => {
    const [open, setOpen] = useState(false);

    return <Component 
             open={open} 
             onToggle={() => setOpen(open => !open)} 
             {...props}
           />;
};

const StatefulCollapseSection = 
      withToggleState(CollapseSection);

const StatefulModal = 
      withToggleState(SomeModalComponent);

This will allow both controlled and stateful components to work together

import { compose, withState, withHandlers } from 'recompose';

type Props = {
    count: number;
    increment(): unknown;
    decrement(): unknown;
};

const PureCounter = ({ count, increment, decrement }: Props) => (
    <div>
        {count}
        <button onClick={increment}>+</button>
        <button onClick={decrement}>-</button>
    </div>
);

const withCounter = compose<Props, {}>(
    withState('counter', 'setCounter', 0),
    withHandlers({
        increment: ({ setCounter }) => () => setCounter(n => n + 1),
        decrement: ({ setCounter }) => () => setCounter(n => n - 1),
    }),
);

const StatefulCounter = withCounter(PureCounter);

✅ or create HOC with Recompose

With recompose helpers

you can create HOCs easily

3️⃣

Squash logic to hook

const FileUploader = ({ onUpload }) => {
    const inputRef = useRef<HTMLInputElement | null>(null);
    const [selectedFile, setSelectedFile] = useState();
    const processFile = (file: File) => {
        // Some file logic
    };
    const onFileUploadRequested = () => {
        inputRef.current.click();
    };
    const handleFileUpload = (event: ChangeEvent<HTMLInputElement>) => {
        setSelectedFile(processFile(event.target.files[0]));
    };

    const onSubmit = () => {
        onUpload(selectedFile);
    };

    return (
        <div>
            <button onClick={onFileUploadRequested}>Upload</button>
            <input type="file" hidden ref={inputRef} onChange={handleFileUpload}/>
            <button onClick={onSubmit}>Submit</button>
        </div>
    );
};

❌ Instead of this

Sometimes view require a lot of boilerplate to make it work

const useFileUpload = () => {
    const inputRef = 
          useRef<HTMLInputElement | null>(null);
  
    const [selectedFile, setSelectedFile] = 
          useState();
  
    const processFile = (file: File) => {
        // Some file logic
    };
    const onFileUploadRequested = () => {
        inputRef.current.click();
    };
    const handleFileUpload = 
          (event: ChangeEvent<HTMLInputElement>) => {
        setSelectedFile(processFile(event.target.files[0]));
    };

    return {
      inputRef, 
      selectedFile, 
      setSelectedFile, 
      onFileUploadRequested, 
      handleFileUpload,
    };
};

✅ Try squashing to hook

const FileUploader = ({ onUpload }) => {
    const { onFileUploadRequested,
           selectedFile, inputRef,
           handleFileUpload } = useFileUpload();

    const onSubmit = () => {
        onUpload(selectedFile);
    };

    return (
        <div>
            <button 
               onClick={onFileUploadRequested}>
                 Upload
            </button>
            <input 
              type="file" hidden ref={inputRef} 
              onChange={handleFileUpload}
            />
            <button 
              onClick={onSubmit}>
            Submit
           </button>
        </div>
    );
};

You can hide some code in Hook

and expose only public fields

4️⃣

Extract jsx to smaller functions

export const SomeBiggerComponent = ({someProp}) => {
    return (
        <div>
            <div>
                <h1>Some headline</h1>
            </div>
            <div>
                <p>Some text</p>
            </div>
            {someProp ? (
                <div>Some optional content rendered conditionally</div>
            ) : (
                <div>Or another content rendered otherwise</div>
            )}
            <div>
                {/* And more, and more, and more */}
            </div>
        </div>
    )
}

❌ Instead of this

Code nesting and ternaries in big JSX templates can be hard to read

export const SomeBiggerComponent = ({ someProp }) => {
    const renderContentA = () => (
        <div>Some optional content rendered conditionally</div>
    );

    const renderContentB = () => (
        <div>Or another content rendered otherwise</div>
    );
    const renderContent = () => {
        return someProp ? renderContentA() : renderContentB();
    }

    return (
        <div>
            <div>
                <h1>Some headline</h1>
            </div>
            <div>
                <p>Some text</p>
            </div>
            {renderContent()}
            <div>
                {/* And more, and more, and more */}
            </div>
        </div>
    );
};

✅ Try this

Extracting jsx to partials

give them semantic value (names) and are easier to move to components later

5️⃣

Group the same components' props

import { TextField } from '@material-ui/core';

export const Form = () => (
    <form>
        <TextField
            className="input"
            variant="outlined"
            size="large"
            label="Email"
        />
        <TextField
            className="input"
            variant="outlined"
            size="large"
            label="Password"
        />
        <TextField
            className="input"
            variant="outlined"
            size="large"
            label="First Name"
        />
        <TextField
            className="input"
            variant="outlined"
            size="large"
            label="Last Name"
        />
    </form>
);

❌ Instead of this

We waste at least 9 lines of code by duplication same props

import { TextField } from '@material-ui/core';
import { mapProps } from 'recompose';

const MyTextField = mapProps<TextFieldProps, TextFieldProps>(props => ({
    className: 'input',
    variant: 'outlined',
    size: 'large',
    ...props
}))(TextField);

export const Form = () => (
    <form>
        <MyTextField
            label="Email"
        />
        <MyTextField
            label="Password"
        />
        <MyTextField
            label="First Name"
        />
        <MyTextField
            label="Last Name"
        />
    </form>
);

✅ Try to map props

We can create component with

mapped props to avoid duplication

6️⃣

useEffect instead of lifecycle methods

💡

useEffect synchronize with state changes instead controlling frameworks lifecycle

class Component extends React.Component {
    componentDidMount() {
        // Handle DOM etc
    }

    componentDidUpdate(prevProps) {
        if(prevProps.someProp !== this.props.someProp) {
            // Handle prop changes and do something
        }
    }

    componentWillUnmount() {
        // Clear after component
    }

    render() {
        return <></>
    }
}

❌ Instead of this

Lifecycle methods require us to understand React "internals"

const Component = (props) => {
    useEffect(() => {
        // Handle DOM etc

        return () => {
            // Cleanup after component
        }
    }, []);

    useEffect(() => {
        // Handle prop changes and do something
    }, [props.someProp]);

    return <></>
}

✅ Try Hooks

With hooks we sync to state changes and focus on our data

7️⃣

Move logic to sagas

⏱ Sagas are great not only to handle async operations

🙌 Perfect spot to abstract away business logic from view

👮‍♂️ Logic in Sagas enhances CQRS and event driven architecture

🥰 Sagas improves code reusability (eg. shared threads in React and React Native)

import { loginFormSubmitted } from './login-form-actions';

const LoginForm = () => {
    const dispatch = useDispatch();

    const handleSubmit = (email, password) => {
        dispatch(
            loginFormSubmitted({ email, password }),
        );
    };

    return (
        <form onSubmit={handleSubmit}>
            {/* Some form HTML */}
        </form>
    );
}

✅ It's ok!

Component just emit what happened in its context.

import { put, take, call } from '@redux-saga/effects';
import { push } from 'connected-react-router';

import { Routes } from '../../routes';
import { loginFormSubmitted } from './login-form-actions';
import { apiLoginRequested, apiLoginSucceed } from '../api/login/actions';
import {storageService} from '../../storage/storage-service'

export function* loginUserSaga(action: ReturnType<typeof loginFormSubmitted>) {
    const { email, password } = action.payload;

    yield put(apiLoginRequested(email, password));

    const loginResultAction = yield take(apiLoginSucceed);

    // Storage can be in saga too! Loose coupling FTW
    yield call(storageService, 'session', loginResultAction.payload);

    yield put(
        push(Routes.APP),
    );
}

✅ Now try saga

Saga react's on events and control

business/app logic

8️⃣

Use slots

const Nav = ({ isCheckout }) => {
    if (!isCheckout) {
        return (
            <nav>
                <div>
                    <Logo/>
                </div>
                <div>
                    <Menu/>
                </div>
            </nav>
        );
    // We don't want menu in checkout 
    } else {
        return (
            <nav>
                <div>
                    <Logo/>
                </div>
            </nav>
        );
    }
};

❌ Instead of this

We don't want component to know about where they are placed.

const Nav = ({ leftSlot = null, centerSlot = null, rightSlot = null }) => {
    return (
        <nav>
            {leftSlot && <div>{leftSlot}</div>}
            {centerSlot && <div>{centerSlot}</div>}
            {rightSlot && <div>{rightSlot}</div>}
        </nav>
    );
};

const StandardNav = () => (
    <Nav leftSlot={<Logo/>} rightSlot={<Menu/>}/>
);

const CheckoutNav = () => (
    <Nav centerSlot={<Logo/>} />
);

✅ Try this

We can compose different variants using slots

9️⃣

Use render props

👮‍♂️ Can cause performance issues

Render prop pattern

🍾 Component can pass data to parent's function

🤝 Component accept function as a prop

🤼‍♂️ Useful to decouple UI from state or other data

⚒️ Very useful for utilities and UI libraries

import ReactFlagsSelect from 'react-flags-select';

export const Form = () => {
    return (
        <ReactFlagsSelect
            searchable={true}
            searchPlaceholder="Search for a country"/>
    );
};

❌ Instead of this

🤬

This component force me to use 

its ugly UI

interface Props {
    countries?: string[];
    blackList?: boolean;
    customLabels?: {[propName: string]: string};
    selectedSize?: number;
    optionsSize?: number;
    defaultCountry?: string;
    placeholder?: string;
    className?: string;
    showSelectedLabel?: boolean;
    showOptionLabel?: boolean;
    alignOptions?: string;
    onSelect?: (countryCode: string) => void;
    disabled?: boolean;
    searchable?: boolean;
}

❌ ...and this

Its props don't let me style it

how I need

import ReactFlagsSelect from 'react-flags-select';

import { MyListItem, MyInput } from '@app/components';

export const Form = () => {
    return (
        <ReactFlagsSelect
            renderInput={inputProps => <MyInput {...inputProps} />}
            renderFlag={({ flagImage, countryName }) => (
                <MyListItem image={flagImage} label={countryName}/>
            )}
        />
    );
};

✅ They should've done this

With render props I can provide my own UI with data exposed from component

🔟

Move conditional rendering to composable branches.

const Component = ({ componentData }) => {
    // We have to check every component...
    if (!componentData) {
        return <Loader/>;
    }

    return <div>{componentData.something}</div>;
};

❌ Instead of this

We are not DRY - we need to check every component if data exist

import { branch, renderComponent } from 'recompose';

const Component = ({ componentData }) => {
    // Now SRP - just dumb render
    return <div>{componentData.something}</div>;
};

const hasRequiredData = props => Boolean(props.componentData);

export const ComponentWithLoading = branch(
    // If test pass
    props => !hasRequiredData(props),
    // run this:
    renderComponent(Loader)
    // Otherwise just pass Component
)(Component);

✅ Try this

We recompose branch HOC we can

move this check to other level

🧐 "Prefer duplication over the wrong abstraction"

💵 "Duplication is far cheaper than the wrong abstraction" 

⛔️ "AHA Programming = Avoid Hasty Abstractions"

👍 "Optimize for change first"

Bad abstractions cause problems

Rule of thumb

Remember that you should refactor

when you see it's needed. Don't do it prematurely!

❗ Remember ❗

💪 Use Typescript

Summary

⚒️ Use helpful utils like Recompose

🏑 Use new features like Hooks API

🥇 Try to make your components single-responsibility

🎠 Embrace good event-driven architecture with Saga

📈 Decouple business from UI

🧮 Embrace functional programming patterns