Add Cat Lovers App with React, Redux, Tailwind, and BFF Integration#34
Add Cat Lovers App with React, Redux, Tailwind, and BFF Integration#34melnikolaidis86 wants to merge 24 commits into
Conversation
…s added functionality to remove and add to favourites
…example files and README.md
There was a problem hiding this comment.
Bug: Null Assertion Risks in User and Category Handling
Unsafe non-null assertions (!) are used on catId and userId. catId is an optional parameter, but catId! is used when adding a favourite without a null check. userId can be null from the user context, but userId! is used when fetching favourites without a null check. This can lead to runtime errors if these variables are undefined or null.
src/hooks/useFavourites.ts#L46-L49
platform-react-challenge/src/hooks/useFavourites.ts
Lines 46 to 49 in 4edb84a
Bug: Null Country Code Causes TypeError
Potential TypeError when currentBreed.country_code.toUpperCase() is called. This occurs if currentBreed exists but currentBreed.country_code is null or undefined. The current currentBreed && ... check is insufficient as it only validates currentBreed. Use optional chaining: currentBreed?.country_code?.toUpperCase().
src/components/BreedDetailsModal.tsx#L8-L9
platform-react-challenge/src/components/BreedDetailsModal.tsx
Lines 8 to 9 in 4edb84a
Bug: Null Assertion Errors in User Functions
The handleRemove function uses an unsafe non-null assertion on userId!. This can cause a runtime error if userId is null or undefined, for instance, if the user context is not initialized. A similar issue exists in the toggleFavourite function.
src/hooks/useFavourites.ts#L58-L59
platform-react-challenge/src/hooks/useFavourites.ts
Lines 58 to 59 in 4edb84a
Was this report helpful? Give feedback by reacting with 👍 or 👎
gpositive
left a comment
There was a problem hiding this comment.
Hey @melnikolaidis86 - Thanks a lot for your effort dude! :)
I've checked your assignment and I have some questions for you:
- Noticed that the same favourites network requests are performed multiple times in some cases (ie: in the cats list page). Any idea on why this is happening and how could you prevent it?
- Seems that you're using
headlessuipackage to have a modal component ready for you. If you were to create one yourself, how would you approach it? - Are there any performance improvements that you've may thought on doing?
- If you had some more time, what would you have done differently?
Again thanks for your submission and looking forward to your answers!
| "name": "catlover", | ||
| "version": "0.1.0", | ||
| "private": true, | ||
| "dependencies": { |
There was a problem hiding this comment.
Noticed that you don't have any devDependencies defined. Is this on purpose?
There was a problem hiding this comment.
There are some devDependencies on package.json
"devDependencies": {
"@types/react-router-dom": "^5.3.3",
"autoprefixer": "^10.4.21",
"concurrently": "^9.2.0",
"cypress": "^14.5.2",
"postcss": "^8.5.6",
"tailwindcss": "^3.4.17"
}
However some of the them could have been moved to devDepenencies
"@testing-library/dom"
"@testing-library/jest-dom"
"@testing-library/react"
"@testing-library/user-event"
"@types/jest"
"@types/node"
"@types/react"
"@types/react-dom"
"react-scripts"
"typescript"
"web-vitals"
| @@ -0,0 +1,2 @@ | |||
| CAT_API_KEY= | |||
| MAX_FAVOURITES=10 No newline at end of file | |||
There was a problem hiding this comment.
May you provide some more info about this env variable?
Why do we need a MAX_FAVOURITES variable in the server layer, in the app layer?
Also: What's the rational behind this decision?
There was a problem hiding this comment.
The reason behind this is that there is a hard limit of 100 favorites when fetching favorites and if there are too many favorites I think I should have added pagination on this page too for better ux. However, on second thought I think it would be have been better if I had completely removed this env variable on server and added a hard limit of 100 in FE while checking if the added favorite exists on unique favorites list. Also I think it would have been better if I had added this list on local storage and invalidated when adding a new favorite.
Hey @gpositive - it was a pleasure doing this assignment.
|
This pull request adds a complete Cat Lovers web application built using modern React tooling and best practices. The project showcases several key frontend technologies and architectural patterns, including:
🔧 Tech Stack:
✨ Key Features:
This PR includes all the initial implementation for the core UI, routing, state handling, and styling. It serves as the foundation for future feature additions and scalability improvements.
Please review and let me know if any changes are required. 😊