scaffold-eth / scaffold-eth-2

Open source forkable Ethereum dev stack
https://scaffoldeth.io
MIT License
1.15k stars 732 forks source link

Code style guide documentation #670

Open technophile-04 opened 6 months ago

technophile-04 commented 6 months ago

Description :

We recently did a bunch of clean-up / refactoring code styles PRs to conquer #617 and #232.

Let's document / summarise all the styles that we agreed upon, maybe we can create a Github Wiki as suggested by Carlos here but please free to suggest any other places which feel appropriate 🙌

Some summarised comments :

  1. https://github.com/scaffold-eth/scaffold-eth-2/issues/232#issuecomment-1464059179
  2. https://github.com/scaffold-eth/scaffold-eth-2/issues/232#issuecomment-1465244271
  3. https://github.com/scaffold-eth/scaffold-eth-2/issues/617#issue-2003940907
carletex commented 6 months ago

Yep, I like the Wiki idea... where we can write some (meta) stuff about SE-2 dev. But open to other solutions!

technophile-04 commented 2 months ago

Just tried writing a very basic POC of wiki covering most of the discussion :


Demo : # Code style guide Welcome to Scaffold-ETH 2 wiki! This wiki holds a summary code style guides that are used in the Scaffold-ETH 2 codebase. The code style is inspired by [Google style guide](https://google.github.io/styleguide/tsguide.html) but deviates in some aspects. ### Identifiers Identifiers follows the casing convention in below table: | Style | Catergory | | ---------------- | ---------------------------------------------------------------------------------------------------------------------- | | `UpperCamelCase` | class / interface / type / enum / decorator / type parameters / component functions in TSX / JSXElement type parameter | | `lowerCamelCase` | variable / parameter / function / property / module alias | | `CONSTANT_CASE` | constant / enum / global variables | | `snake_case` | for hardhat deploy files | ### Import Paths Scaffold-ETH 2, nextjs package has path alias `~~` for root of `nextjs` package. While importing use the path alias whenever possible. Example: ```ts import { Address } from "~~/components/scaffold-eth"; import { useScaffoldWriteContract } from "~~/hooks/scaffold-eth"; ``` ### Creating Page Define the page component and then export it as default. ```jsx import type { NextPage } from "next"; const Home: NextPage = () => { return
Home
; }; export default Home; ``` ### Comments Make comments that actually add information. Example: ```ts // BAD: /* * Checks if an address is zero address * @ returns boolean * */ export const isZeroAddress = (address: string) => address === ZERO_ADDRESS; // GOOD: // Checks if an address is zero address export const isZeroAddress = (address: string) => address === ZERO_ADDRESS; ``` ## Typescript ### Types vs Interfaces Scaffold-ETH 2 uses types over interfaces for defining custom types whenever possible. ### Types naming Types should follow the UpperCamelCase convention. custom type should **not** use the `T` prefix for type names. Prefix are used for generic types. Example: ```ts type TAddress = string; // Bad: T prefix is used for generic types type Address = string; // Good ``` ### Type Inference Try to avoid explicitly typing variables unless it's necessary. Example: ```ts const x: boolean = true; // Bad: Unnecessary type annotation, TypeScript can infer the type ```

rin-st commented 2 months ago

Great start Shiv, thanks! Had the same todo in my notes, glad I don't need to write it since 100% you did it better 😄

Make comments that actually add information.


// GOOD:

// Checks if an address is zero address export const isZeroAddress = (address: string) => address === ZERO_ADDRESS;

I think comment doesn't add information here since it copies text from variable name. Probably it's better to use for example (similar, but imho not so obvious)

// GOOD:

const scaffoldConfig = { // The networks on which your DApp is live targetNetworks: [chains.hardhat], }


---
> Define the page component and then export it as default.

It's just won't work if it's not default, so looks like this point shouldn't be in style guide

---
> Did I forget to add something important?

I think no 🤷‍♂️ 

> Should we divide wiki in nextjs and hardhat part ?

Probably next/foundry parts when we add foundry and it will be some rules we need to mention. I think we don't need to divide for now

> Solidity style guide ?

Afaik, we don't have it (except prettier rules)

> Apart from style guide what other things should we mention ?

Link to prettier/eslint configs? Ask to configure IDE so formatters should work?
damianmarti commented 2 months ago

Just tried writing a very basic POC of wiki covering most of the discussion :

Demo :

Code style guide

Welcome to Scaffold-ETH 2 wiki! This wiki holds a summary code style guides that are used in the Scaffold-ETH 2 codebase.

The code style is inspired by Google style guide but deviates in some aspects.

Identifiers

Identifiers follows the casing convention in below table:

Style Catergory UpperCamelCase class / interface / type / enum / decorator / type parameters / component functions in TSX / JSXElement type parameter lowerCamelCase variable / parameter / function / property / module alias CONSTANT_CASE constant / enum / global variables snake_case for hardhat deploy files

Import Paths

Scaffold-ETH 2, nextjs package has path alias ~~ for root of nextjs package.

While importing use the path alias whenever possible.

Example:

import { Address } from "~~/components/scaffold-eth";
import { useScaffoldWriteContract } from "~~/hooks/scaffold-eth";

Creating Page

Define the page component and then export it as default.

import type { NextPage } from "next";

const Home: NextPage = () => {
  return <div>Home</div>;
};

export default Home;

Comments

Make comments that actually add information.

Example:

// BAD:

/*
 * Checks if an address is zero address
 * @ returns boolean
 * */
export const isZeroAddress = (address: string) => address === ZERO_ADDRESS;

// GOOD:

// Checks if an address is zero address
export const isZeroAddress = (address: string) => address === ZERO_ADDRESS;

Typescript

Types vs Interfaces

Scaffold-ETH 2 uses types over interfaces for defining custom types whenever possible.

Types naming

Types should follow the UpperCamelCase convention. custom type should not use the T prefix for type names. Prefix are used for generic types.

Example:

type TAddress = string; // Bad: T prefix is used for generic types
type Address = string; // Good

Type Inference

Try to avoid explicitly typing variables unless it's necessary.

Example:

const x: boolean = true; // Bad: Unnecessary type annotation, TypeScript can infer the type
  • Did I forget to add something important?
  • Most of our discussion's were mostly around nextjs package of monorepo, so the style guide seems a bit skewed towards frontend.

    • Should we divide wiki in nextjs and hardhat part ?
    • What are patterns, worth mentioning in wiki for hardhat part ? (like having snake_case for deploy files)
    • Solidity style guide ?

We have too little solidity code or hardhat code (and we are planning to change it to Foundry) in SE-2, so I think it is unnecessary now.

  • Apart from style guide what other things should we mention ?

cc @carletex, @rin-st , @damianmarti, @Pabl0cks

thanks @technophile-04 !!! I think this is a really great starting point and we should keep in mind to try to keep it updated to date when we define some new code style or find one missing out here

rin-st commented 2 months ago

found interesting point in Google style guide https://google.github.io/styleguide/tsguide.html#function-declarations. I like it but as I understand most people don't. If you're prefer arrow functions I think we need to add it to our styleguide too