-
Notifications
You must be signed in to change notification settings - Fork 58
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
Improve performance with lazy loading, useCallback, and useMemo #52
Conversation
rendi12345678
commented
Jul 1, 2024
•
edited
Loading
edited
- Implemented lazy loading for React components to reduce initial load time
- Applied useCallback and useMemo hooks to prevent unnecessary re-renders
✅ Deploy Preview for eletypes ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
and optimize your svg image sir |
Thanks Rendi! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just couple of places using useCallback but we need to add the set state into it's dependency chain, otherwise I think they probably won't work.
observed issues:
we losing the auto focus (input), when components are lazy loading with useMemo, can we make the auto focus explicity for each input of the component we touched? another things are due to the useCallBack, I think the error/correct/ css changes are not triggered because of the dependency chain are empty. can test here https://deploy-preview-52--eletypes.netlify.app/
}; | ||
return { correct, incorrect, extra }; | ||
}); | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems need to add currSentence, currInput as dependency, instead of empty.
src/App.js
Outdated
@@ -174,7 +177,7 @@ function App() { | |||
|
|||
return ( | |||
<ThemeProvider theme={theme}> | |||
<> | |||
<Suspense fallback={null}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a fallback component instead of having null? probably a loading indicator to handle the waiting to load status, if we want to use Suspense.
setCurrInput(e.target.value); | ||
hiddenInputRef.current.value = e.target.value; | ||
e.preventDefault(); | ||
}; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto, if we want to use usecallback, I think effectively we need to add the setstate in its dependency chain to make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think that converting handleUpdate with useCallback with this change won't improve too much performance, but let me know if you observed fast renderings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe need some reseach again