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

Repo maintenance improvements #1098

Open
5 of 8 tasks
marcalexiei opened this issue Dec 13, 2024 · 5 comments
Open
5 of 8 tasks

Repo maintenance improvements #1098

marcalexiei opened this issue Dec 13, 2024 · 5 comments

Comments

@marcalexiei
Copy link
Contributor

marcalexiei commented Dec 13, 2024

When opening #1097 I noticed a few things that might be fixed / improved about repository maintenance

1. Double lock files

The project contains both pnpm-lock.yml and yarn.lock.

Proposed solution

pnpm-lock.yml was update more recently so I would remove yarn.lock from version control and update all yarn command across the repository using pnpm

2. format script is not valid

Running pnpm format produce an error because the prettier command is missing the [file/dir/glob ...].

Proposed solution

Update the script adding a . and add not relevant folders / files to .prettierignore

3. pnpm run lint returns an error

Running pnpm run lint produce many errors / warnings.

`pnpm run lint` Output
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.2.0

YOUR TYPESCRIPT VERSION: 5.6.3

Please only submit bug reports when using the officially supported version.

=============

./src/components/ApiFormState.tsx
9:39  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/ApiRefTable.tsx
10:53  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/BeekaiBuilderPage.tsx
13:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/components/BuilderPage.tsx
113:6  Warning: React Hook useEffect has a missing dependency: 'setValue'. Either include it or remove the dependency array.  react-hooks/exhaustive-deps
117:6  Warning: React Hook useEffect has a missing dependency: 'setValue'. Either include it or remove the dependency array.  react-hooks/exhaustive-deps
121:6  Warning: React Hook useEffect has missing dependencies: 'editFormData.required' and 'setValue'. Either include them or remove the dependency array.  react-hooks/exhaustive-deps

./src/components/CodeCompareSection.tsx
5:8  Warning: 'colors' is defined but never used.  @typescript-eslint/no-unused-vars

./src/components/DevToolFeaturesList.tsx
22:12  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/DevTools.tsx
31:12  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any
62:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/components/Form.tsx
32:20  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any
33:15  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any
36:39  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any
141:44  Warning: 'ref' is defined but never used.  @typescript-eslint/no-unused-vars

./src/components/Nav.tsx
298:21  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/components/SortableContainer.tsx
17:13  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/Toggle.tsx
21:5  Error: A form label must have accessible text.  jsx-a11y/label-has-associated-control

./src/components/UseControllerContent.tsx
12:62  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/UseFieldArrayContent.tsx
13:62  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/logic/generateCode.ts
1:27  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./src/components/selectNav.tsx
15:5  Warning: onBlur must be used instead of onchange, unless absolutely necessary and it causes no negative consequences for keyboard only or screen reader users.  jsx-a11y/no-onchange

./src/components/sponsorsList.tsx
14:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
21:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
31:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
41:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
52:11  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/pages/about-us.tsx
167:68  Warning: 'i' is defined but never used.  @typescript-eslint/no-unused-vars
181:25  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/pages/media.tsx
24:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
31:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
39:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
47:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
56:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
63:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
71:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element
79:15  Warning: Using `<img>` could result in slower LCP and higher bandwidth. Consider using `<Image />` from `next/image` to automatically optimize images. This may incur additional usage or cost from your provider. See: https://nextjs.org/docs/messages/no-img-element  @next/next/no-img-element

./src/types/global.d.ts
2:15  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

Proposed solution

Apply the fix for all warnings / errors and maybe also update eslint related dependencies when possible (I would consider updating typescript-eslint to get rid of the warning about typescript supported version)

4. Add lint and format checks to CI

Might be worth adding a CI workflow that runs on push and pull_request events on master branch and executes format and lint scripts?


What do you think?

If you'd like, I can create a PR for each of the previously listed points!


@bluebill1049
Copy link
Member

oh ooops, yes plz

@marcalexiei

This comment was marked as resolved.

@marcalexiei

This comment was marked as resolved.

@bluebill1049
Copy link
Member

All suggested changes PRs are now opened!

Additional improvements:

  • enable typescript strict mode
  • enable additional typescript eslint rules that require that strict mode is enabled
  • update eslint to v9
  • update next to v15

If you wish I can open separate PRs also for above points. (Consider that it might take longer than the previous ones 😅)

Hey @marcalexiei thanks very much for the tidy-up! Those are bonuses not required to have, you don't want to occupy more of your freed time. 🙏 The community should appreciate your effort and time as well. <3

@marcalexiei
Copy link
Contributor Author

marcalexiei commented Dec 16, 2024

Those are bonuses not required to have, you don't want to occupy more of your freed time

That is not problem, If I haven't the time I wouldn't have proposed the additional changes 😂


Those are bonuses not required to have

I'll see what I can do in the upcoming days / weeks!

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

2 participants