Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code style guide documentation #670

Open
technophile-04 opened this issue Dec 20, 2023 · 5 comments
Open

Code style guide documentation #670

technophile-04 opened this issue Dec 20, 2023 · 5 comments

Comments

@technophile-04
Copy link
Collaborator

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. TypeScript styleguide #232 (comment)
  2. TypeScript styleguide #232 (comment)
  3. Consistency codebase refactor #617 (comment)
@technophile-04 technophile-04 changed the title Code style guide Code style guide documentation Dec 20, 2023
@carletex
Copy link
Member

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

@technophile-04
Copy link
Collaborator Author

technophile-04 commented Apr 24, 2024

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 ?
  • Apart from style guide what other things should we mention ?

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

@rin-st
Copy link
Member

rin-st commented Apr 25, 2024

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
Copy link
Collaborator

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
Copy link
Member

rin-st commented Apr 29, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants