Skip to content
Atmosfearful edited this page Aug 8, 2022 · 35 revisions

Contribution & Style Guide

Here are a few disparate thoughts & motivations. The intent here is to help us devs get aligned on the style and quality of our contributions.

Philosophy

Our first priority is, of course, security and correctness. Our app is used by tens of thousands of people to move large sums of wealth.

However, we are also operating as a DAO. The enthusiasm of our community is our greatest strength. For these reasons, we should also prioritize dev happiness. We should avoid bogging ourselves down and making it difficult for newcomers to contribute. We should be able to ship fixes and features quickly.

A11y and i18n is important.

Do not assume that our users are all young healthy English-speaking crypto-natives.

  • Proper screen-readable html attributes and alt text
  • Good color contrast, large text
  • Complete support for tab-navigation

DRY vs WET

  • It's OK to copy-paste sometimes!

Required watching https://overreacted.io/the-wet-codebase/

Avoid dependencies.

This is a difficult area where dev-happiness and security are at odds. With Next.js, TypeScript, Emotion, and Ethers.js, we have everything we need to build anything we want. Please don't go installing react-modal react-button react-tab react-paginate react-blah-blah-blah

Careful exceptions are made for really frustrating stuff like accessible/responsive tooltips (tippy.js) and icons (@mui/icons)

Git Conventions

It should be easy to roll-back, cherry-pick, or revert commits. It should be clear where a feature was added, which PR added it, and which release included it.

NO merge commits!

These can be avoided by using a rebase-merge approach, and by rebasing your feature branch to staging before opening a PR. Production (main) and staging should have identical commit histories.

PRs are squash-merged to staging, and staging is rebase-merged to main.

NO working commits (squash-merge PRs)

Making many small commits are great and appreciated by PR reviewers and are generally a good dev practice. But they should not appear on staging or main branches. Feature branches / PRs are squashed and merged to staging as a single, well-named commit.

Code conventions

Margin Considered Harmful

Most layouts can be accomplished with less code using grid. Use <Stack /> components for vertical spacing, columns for horizontal spacing. This also maps very nicely to Figma auto-layouts, works in RTL, and doesn't require overrides for start/end elements.

https://mxstbr.com/thoughts/margin/

By nesting stacks, you can accomplish a clean consistent layout with very minimal css!

// 💚
.contentContainer,
header,
main {
  display: grid;
}
.contentContainer {
  gap: 3.2rem;
}
header {
  gap: 0.8rem;
}
main {
  gap: 1.6rem;
}
// 💚
<div className="contentContainer">
  <header>
    <h1>{title}</h1>
    <p>{subtitle}</h1>
  </header>
  <section>
    {...firstSection}
  <section>
  <section>
    {...secondSection}
  <section>
</div>

Prefer plain CSS

It's easy to do dynamic and conditional styles with cx (classnames) or in some cases, data-attributes.

// example of conditional className
<button
  className={cx({ disabled: !!props.disabled }, styles.button)}
/>;

// example of data driven attributes
<div
  className={styles.colorChanger}
  data-hex={state.hex}
  data-variant={props.variant}
/>;

Composition > configuration

Props and configs might reduce lines of code, but are difficult to refactor and lead to big complex components.

// ❌ bad, configuration via props

<MyPage
  showHeader
  headerTabs={[...]}
  showFooter
  showFooterLogo
>
  {content}
</MyPage>
// 💚 good, composition of reuseable elements! 👍

<PageContainer>
  <Header>
    <Tab />
    <Tab />
  </Header>
  <Main>{content}</Main>
  <Footer>
    <Logo />
  </Footer>
</PageContainer>

Do not destructure props

This helps to prevent name-clashes, demonstrate where a function or value originates, avoids big blocks of code when a component has many props, and requires less refactoring when things are renamed or removed.

// 💚
const MyButton: FC<Props> = (props) => {
  <button onClick={props.onClick}>{props.children}</button>;
};

Use a onEvent/handleEvent naming convention.

I.e. props start with on and handlers functions start with handle

// 💚
const ClickTracker = (props) => {
  const handleClick = (e) => {
    trackClick(e);
    props.onClick(e);
  };

  return <button onClick={handleClick}>Click Me</button>;
};

Provide a return type for any functions that have a return value.

Example: easy to assume this is a number, but it's not! Explicit return types prevent confusion. For functional React components, use the FC<Props> interface.

// 💚
const getQuantity = (): string => {
  return state.quantity;
};

// 💚
const MyButton = (props): FC<Props> => <button {...props} />;

Prefer params objects over multiple arguments

// 💚
const emitEvent = (params) => {
  props.onEvent({ id: params.id, value: params.value });
};

Prefer named exports over default exports.

See this article for reasons why 'default is bad'.