Code Review — Around React (Sprint №14)

Files are correctly placed in src, blocks, and images folders. CRA was utilized properly, and index.css is securely connected to index.js.

Markup has been excellently ported to JSX inside component returns. Naming conventions are strongly followed: components use PascalCase (nouns), functions use camelCase (verbs), and variables descriptively indicate their data without unclear abbreviations. Hooks are correctly placed at the top level of the components.

Excellent job lifting state from Main to App — Main is properly stateless. CurrentUserContext is exported cleanly from the contexts/ directory, embedded efficiently in App via the Provider, and consumed correctly in Main and Card components to avoid prop-drilling.

Local data updates are implemented flawlessly. For example, toggling likes with card.likes.some() and .map() handles immutability perfectly. EditProfilePopup pre-populating data via useEffect also works exactly as expected.

The / route is accurately protected by the ProtectedRoute HOC, and unauthenticated routes (/signup and /signin) are left unprotected as required. After an authorized login, the user successfully hits the / directory.

No .gitignore file was found in the repository. This is a critical omission, as it risks committing things like node_modules or local .env files. Ensure you create one at the root of the project to ignore these heavy/sensitive directories.

1 | node_modules/
2 | .env
3 | dist/
4 | build/

api.js is incorrectly placed inside src/components/; it should be moved to a dedicated src/utils/ directory. Additionally, ProtectedRoute.js was placed in src/auth/ instead of src/components/. Remember to update your imports across the project after moving them.

1 | import React from 'react';
2 | // Suggested import structure in App.js after moving files
3 | import api from '../utils/api';
4 | import ProtectedRoute from './ProtectedRoute';

When signing out, the JWT must be deleted completely, but localStorage.removeItem('jwt') is never called, meaning the token remains and auto-authenticates the user later. Furthermore, after signing out, the user is redirected to /signup instead of the correct /signin page.

12 | function onSignOut() {
13 |   // Critical: Remove token
14 |   localStorage.removeItem('jwt'); 
15 |   setIsLoggedIn(false);
16 |   // history.push('/signup'); // incorrect
17 |   history.push('/signin'); // Correct redirect
18 | }

In App.js, there are two separate useEffect hooks fetching user data. api.getAppInfo() internally calls getUserInfo(), but a second, separate useEffect calls api.getUserInfo() independently, causing two concurrent requests to the same endpoint on every mount. The independent request should be safely removed.

23 | // Do NOT do this:
24 | React.useEffect(() => { api.getAppInfo().then(...) }, []);
25 | React.useEffect(() => { api.getUserInfo().then(...) }, []); // <- redundant
   | 
27 | // Instead, rely only on the single request:
28 | React.useEffect(() => {
29 |   api.getInitialData()
30 |     .then(([userData, cards]) => {
31 |       setCurrentUser(userData);
32 |       setCards(cards);
33 |     })
34 |     .catch(console.error);
35 | }, []);

The component does not pass the onClose() prop down to PopupWithForm. This leaves the "X" button completely disconnected.

26 | <PopupWithForm
27 |   isOpen={props.isOpen}
28 |   onClose={props.onClose} /* <-- Missing prop must be added */
29 |   onSubmit={handleSubmit}
30 |   name="add-card"
31 | >

handleCardDelete correctly removes the card via API, but the delete button in Card.js fires immediately. The required "Are you sure?" confirmation popup exists in App.js but is missing its isOpen, onClose, and onSubmit properties, so it never opens. Change the delete click handler to open the confirmation popup first.

32 | function handleDeleteClick() {
33 |   // instead of: props.onCardDelete(props.card);
34 |   props.onCardDeleteClick(props.card); // Opens confirmation popup
35 | }

178| <ConfirmDeletePopup 
179|   isOpen={isConfirmPopupOpen} 
180|   onClose={closeAllPopups} 
181|   onConfirm={() => handleCardDelete(cardToDelete)} 
182| />

If you are running Node 17+, the project will fail to start due to OpenSSL legacy provider issues. Consider modifying the "start" script in package.json to fix local development setups.

12 | "scripts": {
13 |   "start": "SET NODE_OPTIONS=--openssl-legacy-provider && react-scripts start"
14 | }

Using a descriptive name like [avatarUrl, setAvatarUrl] or [avatarLink, setAvatarLink] improves readability immensely.

  7| // Before
  8| const [value, setValue] = React.useState('');
   | 
 10| // After
 11| const [avatarUrl, setAvatarUrl] = React.useState('');

The file defines handleUpdateAvatar internally and calls api.setUserAvatar() directly. API calls and state-updating logic should be handled in App.js, passing down a callback function to the popup instead.

106| function handleUpdateAvatar(avatarUrl) {
107|   api.setUserAvatar(avatarUrl).then((data) => {
108|     setCurrentUser(data);
109|     closeAllPopups();
110|   }).catch(console.error);
111| }

 8 | function handleSubmit(e) {
 9 |   e.preventDefault();
10 |   // instead of calling api.setUserAvatar
11 |   props.onUpdateAvatar(avatarRef.current.value);
12 | }

handleSignOut() only serves to call onSignOut(). These could easily be merged into a single function, or you can just pass onSignOut directly to the onClick handler.

20 | // Before
21 | <button onClick={handleSignOut}>Sign Out</button>
   | 
23 | // After
24 | <button onClick={onSignOut}>Sign Out</button>

The token seems to be saved in two places differently: using res.token in Login.js and data.jwt in auth.js. Be careful, as using different response identifiers means one of them will save as undefined. Ensure you are extracting the correct property based on the API response.

34 | // Make sure both use the same matching key from the API response
35 | localStorage.setItem('jwt', data.token); // or data.jwt

Using "./signin" instead of an absolute path "/signin" can produce incorrect redirect URLs depending on what the browser considers the current active path to be.

15 | // Before
16 | <Redirect to="./signin" />
   |
18 | // After
19 | <Redirect to="/signin" />

currentUser is instantiated as React.useState(false). Since child components attempt to render currentUser.avatar and currentUser._id, rendering before the API finishes loading will cause undefined property access errors. Initialize it as {}.

21 | // Before
22 | const [currentUser, setCurrentUser] = React.useState(false);
   | 
24 | // After
25 | const [currentUser, setCurrentUser] = React.useState({});