drapisarda / serenegrove

Webapp that allows the user to easily create a custom guided meditation routine.
https://serenegrove.vercel.app
1 stars 0 forks source link

Readme.md, package.json and commit messages suggestions #3

Open leandroinacio opened 7 months ago

leandroinacio commented 7 months ago

Hey sir!

Readme could use the instructions on how to install, run and test the app. I know most of us can see this directly from package.json, but it can be helpful for juniors. I this site, it already recommends a bunch of sections: https://readme.so/editor

One of the things not clear is, which node version should we use to run this project. I suggest having it in the docs, but also having a file/configuration that allows us to just run a command to setup which version to use. Famous tools for this are https://github.com/nvm-sh/nvm and https://docs.volta.sh/guide/

leandroinacio commented 7 months ago

About the commit messages - I'd go with either https://udacity.github.io/git-styleguide/ or our old friend https://gitmoji.dev/

leandroinacio commented 7 months ago

Some suggestions I found when checking your package.json: https://www.serenegrove.com/ -> could use caching (not sure what you're using for hosting, but infra could get documented in readme.md as well)

I would add a linter and husky to handle pre-commit/pre-push linting, type-checking and testing.

leandroinacio commented 7 months ago

After checking your nuxt.config.ts:

Is the app SSG or SSR? I see SSR in the config file, but there is no payload, no customization... so I wonder why you made this choice.

This code snippet is there, but I'm not sure why:

test1: process.env.TEST_1,

I also did not find any env.ts or config.ts so I don't know where these variables in process.env are coming from.

I will continue reviewing later.

drapisarda commented 3 months ago

Thanks, Man. I'm going through all of them!

drapisarda commented 3 months ago

✅ .nvmrc Husky Lint Prettier e2e tests for the main user interactions SSR is now off Better commit messages Types fixed ⏳ A full test cover of the components is in the making (97% right now).

leandroinacio commented 1 month ago

Hey sir! I'm back with a couple more suggestions.

File structure

  1. Remove .nvm (.nvmrc is enough)
  2. Commit your package-lock.json - it's industry standard and suggested by the npm docs - also it will allow us to use npm ci
  3. Why not add a sitemap.xml? There is an official module for it.
  4. Add a browserslist key in your package.json to better handle your browser management with esbuild.
  5. Add a .browserslistrc to your root (if any dependency is using babel - I haven't checked, so maybe you won't even need it)

Security

  1. You have two high security vulnerabilities - try npm audit fix

Lighthouse

  1. A lot of these components you have can be rendered server side and then all their JS can be discarded. That would make your final bundle smaller and your performance better. You can do that with NuxtIsland. It also decreases hydration time.
  2. If a lot of JS still runs in the browser, you should try Partytown. But based on the page size, it shouldn't be required.
drapisarda commented 3 weeks ago

Thank you, man. I went through most of them. I'll check on NuxtIsland and Partytown ASAP. The sitemap module breaks the TS typecheck. I'll check how to implement it ASAP