npm install
was broken because the dependencynode-sass
. changing to thesass
LTS fixed it.create-react-app
is no longer recommended by the React team. for the next SPA we build, we can consider usingVite
instead. this will provide better compatibility with commonly-used libraries and might avoid future bugs.- the implementation doesn't satisfy the user stories, I was unable to search for movies or see a list of movies in the home page. currently i see only an empty state.
- the fetch url is incorrect, this can be improved by using a single source of truth (from constants.js, for example) and testing your code before opening a pull request.
- there is no linter configured, which results in irregular code styles in the codebase (e.g.: both single and double quotes, uneven tab spaces, lack of semi-colons, etc...). it would also prevent other issues in your code, such as unused variables, unused functions, and empty functions.
- there is room to improve in naming of variables and functions, always opt for a more descriptive name. for example: you could rename
myClickFunction
tohandleCloseModalClick
. - sensitive information exposed in
constants.js
, prefer environment variables. it is fundamental to add.env
to.gitignore
to not push sensitive information to public repositories. - the TMDB
api_key
leaks in the URL, which can remain in browser history and be exploited. using the TMDB Bearer Token through an HTTP Header may be a safer approach.x - there are variables declared with var, prefer scoped declarations such as const and let.
- there are deprecated methods being used (such as cancel-bubble).
- there are inline styles being mixed with scss, prefer scss. this should be avoided because it can lead to specificity issues if we try to apply styles to the same element.
- prefer to break down your changes into smaller Conventional Commits. this would make it easier to understand your PR and review your code appropriately.
- the folder structure could be improved by using modules, where each module is a folder for its own jsx, styles, and tests (also mocks and api calls, if they exist). See this template example from the redux team
- prefer a debounce on the SearchMovie component, since it is sending a request after every keystroke, which may lead to being rate-limited and a worse experience, especially if the user has a slow network (4g)
- some dependencies could be replaced by native HTML implementations that are simpler, more accessible, and reduce the bundle size. example: prefer a
<dialog>
component overreact-popup
and an<iframe>
overreact-player
.
Created
September 1, 2024 18:57
-
-
Save LukeberryPi/2833f7f21e2cb102c43bc6406db17749 to your computer and use it in GitHub Desktop.
my code review for a coding assignment (movieland)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment