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

chore(eslint): apply eslint fixes and remove unused codes #1044

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Jun 2, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Related to #941

Description of the change:

Chores

  • Remove unused codes.
  • Apply eslint fixes.
  • Use FC alias for FunctionComponent (perhaps code base can be a little bit smaller?).

Fixes

  • Fixed infinite effect loop error by removing effect that open the problem section.
    • How to reproduce: launch cryostat only with run.sh, observe the problem notification, and the browser console will show error with title Warning: Maximum update depth exceeded.
    • Fix by remove buggy effect and open the problem by default. This fix also allow users to close the problem notification section if needed (previously not possible).

Motivation for the change:

Regular "house-keeping" for codebase.

@tthvo tthvo added the chore Refactor, rename, cleanup, etc. label Jun 2, 2023
@tthvo tthvo requested a review from andrewazores June 2, 2023 06:45
@mergify mergify bot added the safe-to-test label Jun 2, 2023
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1044-e8f746587e38972e1ec54b6bc617f157a42b3fa9 sh smoketest.sh

@tthvo tthvo marked this pull request as draft June 2, 2023 07:04
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1044-9be65cb329c099223b6b6f124184ae3835dbda07 sh smoketest.sh

@tthvo tthvo marked this pull request as ready for review June 2, 2023 07:26
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1044-107717d6c4e7dde0fe91ebbcc10f9a6668f6cc02 sh smoketest.sh

@tthvo tthvo added the fix label Jun 2, 2023
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1044-bb5a721d9a3ba4f0661263b4e11c4f7cae190cea sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Jun 2, 2023

Seems like if the user chooses minimal deployment, the infinite effect loop error will cause the web-client to be significantly slow. Should be backport this (maybe split the fix into another PR?)?

@andrewazores
Copy link
Member

How to reproduce: launch cryostat only with run.sh, observe the problem notification, and the browser console will show error with title Warning: Maximum update depth exceeded.

I wasn't able to reproduce this (backend commit: 332fcbee8b7469935b88f1934dae917f24a3d5c5), I just see a warning notification and no error in the console.

@tthvo
Copy link
Member Author

tthvo commented Jun 2, 2023

I wasn't able to reproduce this (backend commit: 332fcbee8b7469935b88f1934dae917f24a3d5c5), I just see a warning notification and no error in the console.

Opps I left out an important step: you need to launch a dev server for front-end to see react warning. Production asset won't show it.

@andrewazores
Copy link
Member

andrewazores commented Jun 2, 2023

I tried:

$ CRYOSTAT_DISABLE_SSL=true CRYOSTAT_CORS_ORIGIN=http://localhost:9000 sh run.sh

in one terminal, and:

$ yarn start:dev

and this seems to reproduce what you're describing. If I click the notification bell to expand the drawer then I get the console errors and cannot collapse the Problem notification section.

@tthvo
Copy link
Member Author

tthvo commented Jun 2, 2023

Okayy! Then this PR fixed it right?

@andrewazores
Copy link
Member

With this PR branch checked out, repeating the steps above seems to fix it - I can open and close the notification drawer as well as the Problems section, and there is no console error.

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1044-156b6a4670d30d25cfe34dffa54fee899a46f9a2 sh smoketest.sh

@andrewazores andrewazores merged commit 2651228 into cryostatio:main Jun 2, 2023
@tthvo tthvo deleted the eslint-fixes branch June 2, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. fix safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants