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.

Sign up for GitHub

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

Jump to bottom

move worker availability request to rtk query #73

Merged
merged 9 commits into from
Jul 20, 2022
Merged

move worker availability request to rtk query #73

merged 9 commits into from
Jul 20, 2022

Conversation

polaroidkidd
Copy link
Contributor

it all looks cleaner now, but the app is still re-rendering everytime the requests are made, even if the returned data is identical.

@polaroidkidd polaroidkidd force-pushed the fix/worker-availability-request-rerenders-entire-app branch 4 times, most recently from 9b86e8b to a0af782 Compare July 14, 2022 17:03
@polaroidkidd
Copy link
Contributor Author

I also removed a lot of console.log statements which were polluting the console and switched out console.log for console.error in the catch blocks.

@polaroidkidd polaroidkidd force-pushed the fix/worker-availability-request-rerenders-entire-app branch from a0af782 to 200be35 Compare July 14, 2022 20:23
@polaroidkidd polaroidkidd marked this pull request as ready for review July 14, 2022 20:27
@derneuere
Copy link
Member

Hmm, when I wanted to try it out I ran into this issue: welldone-software/why-did-you-render#85
I thought that maybe my node_modules folder was faulty and deleted it. Now I can't install the dependencies at all without running into a semantic-ui issue, which I think was already fixed 😅 Semantic-Org/Semantic-UI#7073

Can you reproduce these issues or are they related to my setup?

@polaroidkidd
Copy link
Contributor Author

whoops, yes, I'm also having those issues. I'm going to resolve them this weekend.

@derneuere
Copy link
Member

Alright, I could fix the semantic-ui issue by force cleaning the npm cache. Not sure why that happened.

@polaroidkidd polaroidkidd force-pushed the fix/worker-availability-request-rerenders-entire-app branch 4 times, most recently from d8a5d44 to 9807cc7 Compare July 18, 2022 21:41
@polaroidkidd
Copy link
Contributor Author

managed to fix it, mostly. Still have some components to migrate, but getting there!

@polaroidkidd
fixed bug where entire app rerenders on every worker-availabillity re…
9639232 
…quest

   * replaced connected-react-router with redux-first-history as connected-react-router appears to be no longer maintained and that's there the bug originated from
   * refactored Worker-Availability component
   * updated dependencies
@polaroidkidd
migrate to react-router-dom v6
98d3698 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
suppress prop-spearding warning for loginpage
7a9c1f0 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
fixed the rendering loop within the <Navigate .../> component
7723748 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
migrate to useParams in various components
ac88261 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
fix package-lock.json
a00730f 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd polaroidkidd force-pushed the fix/worker-availability-request-rerenders-entire-app branch from a2fa5ba to a00730f Compare July 19, 2022 15:07
@polaroidkidd
Copy link
Contributor Author

right, all changes should be working now. I still don't know why the "finished" flashes twice when logging in for the first time though

@polaroidkidd
fix type imports
0fe9fe1 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
fix type imports again
cc1719c 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
fix duplicate 'finished' notification
99df305 
Signed-off-by: Daniel Einars <[email protected]>
@polaroidkidd
Copy link
Contributor Author

right, fixed everything. Also, we have an upgrade to react-18 in here now!

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 20 Code Smells

No Coverage information No Coverage information
0.6% 0.6% Duplication

@derneuere
Copy link
Member

Awesome! Works as expected :)

@derneuere derneuere merged commit f7608ab into LibrePhotos:dev Jul 20, 2022
@polaroidkidd polaroidkidd deleted the fix/worker-availability-request-rerenders-entire-app branch July 20, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derneuere derneuere derneuere left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@polaroidkidd @derneuere