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
Refactoring React Components
By lkostrowski
Refactoring React Components
- 1,235