• npm install was broken because the dependency node-sass. changing to the sass LTS fixed it.
• create-react-app is no longer recommended by the React team. for the next SPA we build, we can consider using Vite 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 to handleCloseModalClick.
• 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 over react-popup and an <iframe> over react-player.