Conversation
|
@Sky-A-Fox I see you have a commit message that reads "netlify link added", but I can't find the actual link in your PR. Can you please share it? |
|
|
||
| // Загружаем товары при изменении категории | ||
| useEffect(() => { | ||
| let isMounted = true; |
There was a problem hiding this comment.
Can you explain why you need this isMounted flag?
There was a problem hiding this comment.
I’ve seen this pattern mentioned as a way to avoid state updates after unmount and potential memory leaks in async effects.
That said, I’m aware that it works outside of React’s lifecycle, doesn’t scale well, and doesn’t look great—especially since it’s used in multiple places.
I agree it would be better to replace it with a proper request cancellation (e.g. AbortController).
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| // Используем ref для отслеживания первого рендера | ||
| const isInitialMount = useRef(true); |
There was a problem hiding this comment.
Can you explain why you need this isInitialMount flag?
There was a problem hiding this comment.
I used it because I had problem with the logic. But in general we don't need it because it's a crutch.
|
|
||
| // Показываем загрузку | ||
| if (loading && products.length === 0) { | ||
| return <div style={{ padding: '20px', textAlign: 'center' }}>Loading products...</div>; |
There was a problem hiding this comment.
You use inline styles here and css stylesheets elsewhere. For consistency, it's best to use one or the other.
| const loadCategories = async () => { | ||
| try { | ||
| const response = await fetch("https://fakestoreapi.com/products/categories"); | ||
| if (!response.ok) throw new Error('Failed to load categories'); |
There was a problem hiding this comment.
If you throw a new error in your try block, the message from the error gets passed to the catch block. In your implementation, this means you show the same error mssage twice.
| const loadCategories = async () => { | ||
| try { | ||
| const response = await fetch("https://fakestoreapi.com/products/categories"); | ||
| if (!response.ok) throw new Error('Failed to load categories'); |
There was a problem hiding this comment.
If you throw a new error in your try block, the message from the error gets passed to the catch block. In your implementation, this means you show the same error mssage twice.
There was a problem hiding this comment.
Great work, Ivan!
You have made good fetching code - just what we need. You may have overengineered a bit - be careful not to copy & paste code that you don't necessarily need. Also please delete unused files (e.g. App.css) to keep your files clean for your colleagues.
Also your PRs should include only the files that are relevant to this week, not last week's code. Please make sure you keep your PRs lean - the same will be true at the company you join!
my bad forget to push. |
2 participants
No description provided.