CodingGarden / listd

listd is a Full Stack App that will allow users to create, share and watch lists of YouTube channels. This app is being built LIVE on Twitch https://twitch.tv/codinggarden
https://twitch.tv/codinggarden
MIT License
193 stars 53 forks source link

Let's improve XD (Developer Experience) #131

Closed babakfp closed 11 months ago

babakfp commented 12 months ago

Hi

Here are some recommendations for changes:

Prettier Configuration

https://prettier.io/docs/en/options.html

Remove these config options

useTabs: true,
singleQuote: true,
trailingComma: 'es5',
printWidth: 100,
bracketSameLine: true,
tailwindConfig: './tailwind.config.cjs',
pluginSearchDirs: ['.'],

If we remove any of these options, the default value is going to be used instead.

useTabs (default: false)

Tabs are evil! Please read this: https://alexkondov.com/indentation-warfare-tabs-vs-spaces.

Todo

[ ] Create ./.vscode/settings.json and add this: "editor.insertSpaces": true

singleQuote (default: false)

Use double quotes for consistency between your JavaScript, HTML, and CSS. It's also easier to write strings that contain a single quote!

trailingComma (default: "es5")

"es5" is the default, so there is no need to have this config options.

In v3, "all" is the default, which I think it's better.

Todo

[ ] Remove the trailingComma config option when updating Prettier to version 3.

printWidth (default: 80)

Using 80 is better for small screen sizes like laptops.

bracketSameLine (default: false)

This makes the code look broken, unreadable, and ugly. I'll argue that this is one of the worst coding styles!

tailwindConfig

We don't need this because it doesn't accomplish anything in this project. Please read this: https://github.com/tailwindlabs/prettier-plugin-tailwindcss#options

pluginSearchDirs

I think this option is deprecated or something. It's nowhere to be found in prettier docs. Without it, everything works fine.

Change these config options

semi: false,
tabWidth: 4,
arrowParens: "avoid",

The default value for these options is not desired. So, let's change them.

semi

Why it's a good idea to not to use semicolons:

  1. Readability:

    • Omitting semicolons can lead to cleaner and more readable code by reducing visual clutter.
  2. Consistency:

    • Many modern JavaScript style guides and linters (e.g., ESLint with "semi: never" rule) encourage omitting semicolons, promoting a consistent coding style.
  3. Developer Preference:

    • Some developers prefer the simplicity of not using semicolons and find it more aesthetically pleasing.
  4. Reduced Keystrokes:

    • Omitting semicolons can save typing effort and reduce the risk of typos, especially in large codebases.
  5. Avoiding Pitfalls:

    • Omitting semicolons can help avoid common bugs caused by incorrect semicolon placement or missing semicolons at the end of blocks.
  6. Interoperability:

    • Modern JavaScript engines can handle semicolon insertion automatically, making semicolons optional and improving interoperability with other languages.
  7. Minification Benefits:

    • In the context of minification and code compression, omitting semicolons can lead to smaller file sizes.
  8. Embracing Modern Syntax:

    • With the introduction of ES6 and beyond, JavaScript syntax has become more lenient with semicolons. Omitting them aligns with the latest language features.

tabWidth

By changing the tab width to 4, we can improve code readability and reduce confusion significantly.

Todo

[ ] Create ./.vscode/settings.json and add this: "editor.tabSize": 4

arrowParens

By avoiding arrow function paracentesis we can improve code readability and make the code cleaner.

Use PNPM instead

Todo

[ ] Add "How to install PNPM" to CONTRIBUTING.md.

Pre-Commit hooks

Why it's a bad idea to use pre-commit hooks:

  1. Watch this video: https://www.youtube.com/watch?v=RAelLqnnOp0

  2. Overhead for Developers:

    • Pre-commit hooks introduce additional steps and potentially slow down the development process, leading to frustration for developers.
  3. Risk of False Positives:

    • Overly strict pre-commit hooks can result in false positives, rejecting commits that are actually valid and error-free.
  4. Difficulty for New Contributors:

    • Pre-commit hooks can be intimidating for new contributors, deterring them from participating in open-source projects or collaborative development.
  5. Impact on Code Review:

    • Pre-commit hooks can delay code reviews, as developers may need to address hook-related issues before receiving feedback.
  6. Bypass with --no-verify:

    • Developers can bypass pre-commit hooks by using the --no-verify option when committing, potentially undermining the intended checks and validations.

Finishing talks

All of these are just opinions, feel free to talk about it on the stream, I think it would be nice talk. Thanks ❤️.

Have you checked if this issue has already been raised?

Code of Conduct

w3cj commented 11 months ago

Thanks for the suggestions. I know it took some time to write out why and how things should be changed, and I appreciate that.

  1. Prettier - we are using eslint with the airbnb config. Our prettier config is in line with those style choices. - for the extraneous options, we can remove those.
  2. pnpm - we agreed early on that we don't want to require people to install this since everyone will already have npm
  3. pre-commit hooks - we find them useful and necessary for this project. We use --no-verify if working on a feature branch in progress, but the tests are expected to pass if the code gets pushed to github. The video you linked is mainly advice for working on a team of developers you know and trust. This is an open source project with contributors of all types.