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

refactor: Fix front-end lint issues #347

Merged
merged 7 commits into from
Feb 23, 2022

Conversation

bryphe-coder
Copy link
Contributor

Noticed while running through the build steps (make build) that we were getting some lint warnings that weren't blocking build:

./pages/workspaces/[user]/[workspace].tsx
32:58  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any
32:96  Warning: Unexpected any. Specify a different type.  @typescript-eslint/no-explicit-any

./components/Form/FormCloseButton.tsx
26:6  Warning: React Hook useEffect has a missing dependency: 'onClose'. Either include it or remove the dependency array. If 'onClose' changes too often, find the parent component that defines it and wrap that definition in useCallback.  react-hooks/exhaustive-deps

./components/Navbar/UserDropdown.tsx
38:14  Warning: Unnecessary conditional, value is always truthy.  @typescript-eslint/no-unnecessary-condition
68:10  Warning: Unnecessary conditional, value is always truthy.  @typescript-eslint/no-unnecessary-condition

./components/Redirect.tsx
20:6  Warning: React Hook useEffect has missing dependencies: 'router' and 'to'. Either include them or remove the dependency array.  react-hooks/exhaustive-deps

./components/SignIn/SignInForm.tsx
126:19  Warning: Unnecessary optional chain on a non-nullish value.  @typescript-eslint/no-unnecessary-condition

It turns out our ESLint config wasn't being picked up, so I fixed that (it wasn't properly named). This PR turns warnings-as-errors on, fixes the issues, and also removes the "Project Create" page, because it isn't used at this time.

Wanted to clean this up before on-boarding more FE developers

Comment on lines +21 to +23
const onCancel = useCallback(async () => {
await push(`/projects/${organization}/${projectName}`)
}, [push, organization, projectName])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This had to be moved before the early-return because hooks always have to be called consistently and in a consistent order.

@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #347 (df532d8) into main (2e12cb9) will increase coverage by 0.22%.
The diff coverage is 29.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
+ Coverage   67.64%   67.86%   +0.22%     
==========================================
  Files         147      147              
  Lines        7930     7905      -25     
  Branches       77       72       -5     
==========================================
+ Hits         5364     5365       +1     
+ Misses       2023     1998      -25     
+ Partials      543      542       -1     
Flag Coverage Δ
unittest-go-macos-latest 66.58% <ø> (+0.13%) ⬆️
unittest-go-ubuntu-latest 67.40% <ø> (-0.08%) ⬇️
unittest-go-windows-2022 65.98% <ø> (+0.04%) ⬆️
unittest-js 66.10% <29.41%> (+1.94%) ⬆️
Impacted Files Coverage Δ
site/components/Form/FormCloseButton.tsx 100.00% <ø> (ø)
site/components/Navbar/UserDropdown.tsx 86.95% <ø> (-1.05%) ⬇️
site/jest-runner-eslint.config.js 0.00% <ø> (ø)
...pages/projects/[organization]/[project]/create.tsx 0.00% <0.00%> (ø)
site/pages/workspaces/[user]/[workspace].tsx 0.00% <0.00%> (ø)
site/components/Redirect.tsx 100.00% <100.00%> (ø)
site/components/SignIn/SignInForm.tsx 100.00% <100.00%> (+3.12%) ⬆️
site/contexts/UserContext.tsx 77.27% <100.00%> (ø)
coderd/provisionerdaemons.go 60.20% <0.00%> (-0.60%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e12cb9...df532d8. Read the comment docs.

Copy link
Contributor

@presleyp presleyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup!

@bryphe-coder bryphe-coder merged commit 707016a into main Feb 23, 2022
@bryphe-coder bryphe-coder deleted the bryphe/refactor/fix-lint-issues branch February 23, 2022 04:10
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

Successfully merging this pull request may close these issues.

None yet

3 participants