Would like some feedback. #11
Conversation
made form input work, added useMemo, although did not fix ContentContainer memo
src/ContentContainer.tsx
Outdated
| return primeFactorize().length | ||
| } | ||
|
|
||
| }, [name]); |
There was a problem hiding this comment.
You don't want name in the dependency array for useMemo, this will cause that value to recompute each time the name prop changes and you lose value of the caching. The dependency array can be empty since this only needs to run once on component mount.
| import Display from "./Display"; | ||
| import { Name } from "./App"; | ||
|
|
||
| const DisplayContainer = ({name}: {name: Name}) => { | ||
|
|
||
| const firstName = useMemo(() => `${name.first}`, [name.first]); | ||
| const lastName = useMemo(() => `${name.last}`, [name.last]); |
There was a problem hiding this comment.
The slowness is from the comment I left above since we want first and last name to update on change. The useMemo wound't do anything here since the name prop is updating on each keystroke. When props change the component re-renders no matter what.
| const trending = await response.json(); | ||
|
|
||
| return trending; | ||
| } catch (error) { console.log("you did the error again", error); throw error; |
There was a problem hiding this comment.
While this is valid I think in this case the throw in the catch block is unnecessary. See this: https://www.w3schools.com/js/js_errors.asp
src/App.tsx
Outdated
|
|
||
| const handleNameUpdate=(field: keyof typeof name, newName: string) => { | ||
| // The form input is not updating can you explain why?- react state imutable | ||
| setName((prevState) => { | ||
| prevState[field] = newName |
There was a problem hiding this comment.
you could remove the prevState syntax all together like this
const handleNameUpdate=(field: keyof typeof name, newName: string) => { setName({...name, [field]: newName}) }
To Keep it could look like this
const handleNameUpdate=(field: keyof typeof name, newName: string) => { setName((prevState) => { const newState = {...prevState} newState[field] = newName return newState }) }
removed name from ContentConatiner.tsx so it would not re-render. Removed and updated the handleNameUpdate
Went through and tried to resolve some of the issues. I was able to set up the text box input in the form container, then I saw what you were referring to with the code being slow. I am not sure if that was set up correctly, as it is not using the Form/DisplayContainer in the App.tsx I set up the useMemo hook, although it may not be set up correctly, as the app was still running some redundant code/calculations. I did change some of that in the utils.ts, and I am not sure if that code was supposed to be placeholder or if I was allowed to change that. It changed the important number to 0 so that may not be the case. I also set up some error checking in the api.ts, but I was not sure if I was supposed to be seeing those errors in the console log or not. Some of the changes were more straightforward. Consolidating the interface/type for the Incomplete/CompleteRating seemed like it was the right change.
Most of these fixes I would want to ask someone else if they were appropriate, or if a different change was needed. I know that just because some code works, does not necessarily mean it is the best code to use. Based on feedback, I will most likely try to adjust some of the changes made.