implementation by MateuszSeler · Pull Request #2037 · mate-academy/react_static-list-of-posts

MateuszSeler · 2026-01-04T21:48:34Z

No description provided.

mateacademy-ai-mentor

Review Summary

Hello! You've done a great job structuring the components and implementing the core rendering logic. However, some changes are needed to meet all the project requirements before this can be approved.

There are a few critical issues to address:

Project Structure: All type definitions (Post, User, Comment) must be moved from the component folders to the required ./src/types/ directory. The task explicitly requires this centralized structure for types.
Potential Runtime Error: In src/App.tsx, users.find(...) can return undefined if a user isn't found for a given post. This can crash the application since the Post type expects a User object, not undefined. Please add logic to handle cases where a user might not be found to prevent this error.

Addressing these points will bring your solution into full compliance with the task requirements. Keep up the good work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+import { Post } from './components/PostInfo/Post';
+import { Comment } from './components/CommentInfo/Comment';
+import { User } from './components/UserInfo/User';


According to the task description, all types should be created in the ./src/types/ directory. These type definitions should be moved there from the component folders, and the imports updated accordingly.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+const comments: Comment[] = commentsFromServer;
+const users: User[] = usersFromServer;
+
+postsFromServer.map(postFromServer => {


The map function is intended to create a new array based on the transformation of each element of the original array. Using it here with posts.push to modify an external array is an unconventional use of map. A more idiomatic approach would be to declare and initialize posts with the result of this map operation.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+
+postsFromServer.map(postFromServer => {
+  posts.push({
+    user: users.find(user => user.id === postFromServer.userId),


The find method returns undefined if no user matches the userId. The Post type, however, expects the user property to be of type User, not User | undefined. This could cause a runtime error in child components if a post's user is not found. Please consider how to handle this possibility.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+export interface Comment {
+  postId: number;
+  id: number;
+  name: string;
+  email: string;
+  body: string;
+}


According to the task description, all types should be created in the ./src/types/ directory. This Comment interface should be moved to a file within that directory, for example, src/types/Comment.ts.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+    <div className="CommentInfo">
+      <div className="CommentInfo__title">
+        <strong className="CommentInfo__name">{comment.name}</strong>
+        {' by '}


This violates checklist item #5: 'Use string interpolation inside tag content'. While not strictly interpolation, wrapping static text in {} is unnecessary here. You can write the text directly between components, for example: ...</strong> by <a.... This makes the code cleaner.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+      <div className="PostInfo__header">
+        <h3 className="PostInfo__title">{post.title}</h3>
+        <p>
+          {' Posted by  '}


This violates checklist item #5: 'Use string interpolation inside tag content'. For static text like this, you can write it directly inside the <p> tag without the curly braces and quotes. For example: <p>Posted by <UserInfo ... /></p>.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+export interface User {
+  id: number;
+  name: string;
+  username: string;
+  email: string;
+}


According to the task description, all types should be created in the ./src/types/ directory. This User interface should be moved to a file within that directory, for example, src/types/User.ts.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

+export interface Post {
+  user: User;
+  id: number;
+  title: string;
+  body: string;
+  comments: Comment[];
+}


According to the task description, all types should be created in the ./src/types/ directory. This Post interface should be moved to a file within that directory, for example, src/types/Post.ts.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

@@ -1,3 +1,11 @@
 import React from 'react';
+import { Post } from '../PostInfo/Post';


According to the task description, all types should be created in the ./src/types/ directory. The Post type should be moved to a file within that directory (e.g., src/types/Post.ts) and imported from there.

mateacademy-ai-mentor · 2026-01-04T21:50:28Z

@@ -1,3 +1,14 @@
 import React from 'react';
+import { User } from './User';


According to the task description, all types should be created in the ./src/types/ directory. The User type should be moved to a file within that directory, and this import should be updated to point to it.

mateacademy-ai-mentor

Review Summary

Great job addressing the major issues from the last review, especially with moving the types and handling missing users! The application is much more robust now. However, a few changes are needed to meet all the project requirements before this can be approved.

The most critical issue is that the styles have not been split into component-specific files as required by the task. Please move the relevant styles from App.scss into files like PostList.scss, UserInfo.scss, etc.

Additionally, please address these checklist violations:

In App.tsx, the findUser function should be renamed to findUserById to be more descriptive [CHECKLIST ITEM #6].
In PostInfo.tsx and CommentInfo.tsx, please use template literals for strings like {' by '} to comply with [CHECKLIST ITEM #5].

Lastly, for best practices, please wrap the returned arrays in PostList.tsx and CommentList.tsx with a React Fragment (<>...</>) and refactor the .map() in App.tsx to return a new array instead of using .push().

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-01-05T17:10:08Z

+const comments: Comment[] = commentsFromServer;
+const users: User[] = usersFromServer;
+
+function findUser(usersToCheck: User[], id: number): User {


This violates checklist item #6: 'If you are searching some entity by some filter, specify it in function/method name'. Since this function finds a user by their id, a more descriptive name would be findUserById.

mateacademy-ai-mentor · 2026-01-05T17:10:08Z

+  return user;
+}
+
+postsFromServer.map(postFromServer => {


Using Array.prototype.map() for side effects like posts.push() is generally considered an anti-pattern. The purpose of .map() is to create a new array from an existing one. A better approach is to use .map() to return a new array and assign it directly to the posts constant. This avoids mutating an external array and makes the code more declarative.

mateacademy-ai-mentor · 2026-01-05T17:10:08Z

+  return comments.map(comment => (
+    <CommentInfo key={comment.id} comment={comment} />
+  ));


While React can render an array of elements, a component typed as React.FC is expected to return a single ReactElement or null. Returning an array directly from .map() doesn't conform to this type signature.

To ensure your component returns a single valid element, please wrap the result of the .map() function in a React Fragment (<>...</>).

mateacademy-ai-mentor · 2026-01-05T17:10:08Z

+};
+
+export const PostList: React.FC<Props> = ({ posts }) => {
+  return posts.map(post => <PostInfo key={post.id} post={post} />);


A component typed as React.FC should return a single ReactElement or null. Returning an array of elements directly from .map() doesn't strictly adhere to this type. To fix this, you can wrap your mapped list in a React Fragment (<>...</>), which provides a single root element without adding an extra node to the DOM.

mateacademy-ai-mentor

Review Summary

I am approving this pull request as an educational override to help you progress, but please review the feedback below for future tasks. You've done an excellent job splitting the styles into component-specific files and improving the project structure!

Here are a few key areas for improvement:

Component Encapsulation: In PostList.tsx and CommentList.tsx, it's best practice for list components to render their own container <div>. This makes them self-contained and easier to style.
Using .map() correctly: In App.tsx, .map() should be used to return a new array, not to push items into an existing one. The correct approach is more declarative: const posts = postsFromServer.map(...).
String Interpolation: Remember to use template literals like {` by `} instead of {' by '} in PostInfo.tsx and CommentInfo.tsx for consistency [CHECKLIST ITEM #5].

Keep up the great work, and try to apply this feedback in your next tasks!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-01-05T17:23:52Z

+postsFromServer.map(postFromServer => {
+  posts.push({
+    user: findUserById(users, postFromServer.userId),
+    id: postFromServer.id,
+    title: postFromServer.title,
+    body: postFromServer.body,
+    comments: comments.filter(comment => comment.postId === postFromServer.id),
+  });
+});


While this works, using .map() to cause side effects (like .push() into an external array) is considered an anti-pattern. The purpose of .map() is to create and return a new array.

It would be better to define the posts constant directly with the result of the map function, like this:

const posts: Post[] = postsFromServer.map(postFromServer => ({ user: findUserById(users, postFromServer.userId), // ... other properties }));

This avoids mutating an array and makes the code more declarative.

mateacademy-ai-mentor · 2026-01-05T17:23:52Z

+    <div className="CommentInfo">
+      <div className="CommentInfo__title">
+        <strong className="CommentInfo__name">{comment.name}</strong>
+        {' by '}


This violates checklist item #5: 'Use string interpolation inside tag content'. As mentioned in the previous review, please use a template literal for string fragments like this. For example: {` by `}.

mateacademy-ai-mentor · 2026-01-05T17:23:52Z

+  return comments.map(comment => (
+    <CommentInfo key={comment.id} comment={comment} />
+  ));


This component returns an array of CommentInfo components, which is valid. However, the styles for .CommentList (defined in CommentList.scss) are currently applied by a div in the parent PostInfo component.

For better encapsulation, the CommentList component should render its own root element with className="CommentList". This change will make the component self-contained and ensure that the "No comments yet" message also appears inside the styled container, which is not the case currently.

mateacademy-ai-mentor · 2026-01-05T17:23:52Z

+};
+
+export const PostList: React.FC<Props> = ({ posts }) => {
+  return posts.map(post => <PostInfo key={post.id} post={post} />);


While returning an array of components is valid, it's better for a list component like this to render its own container element (e.g., a <div>). This practice improves component encapsulation, making it easier to style and manage. Please wrap the returned array in a container element.

mateacademy-ai-mentor · 2026-01-05T17:23:52Z

+      <div className="PostInfo__header">
+        <h3 className="PostInfo__title">{post.title}</h3>
+        <p>
+          {' Posted by  '}


This violates checklist item #5: 'Use string interpolation inside tag content'. As mentioned in the previous review, it's better to use a template literal for consistency and readability. Please change this to {` Posted by `}.

implementation

ada2099

mateacademy-ai-mentor suggested changes Jan 4, 2026

View reviewed changes

MateuszSeler force-pushed the develop branch from 6ba161b to a2229c1 Compare January 5, 2026 17:05

fix

4a3d291

MateuszSeler force-pushed the develop branch from a2229c1 to 4a3d291 Compare January 5, 2026 17:06

MateuszSeler requested a review from mateacademy-ai-mentor January 5, 2026 17:07

mateacademy-ai-mentor suggested changes Jan 5, 2026

View reviewed changes

one more fix

726cc38

MateuszSeler requested a review from mateacademy-ai-mentor January 5, 2026 17:21

mateacademy-ai-mentor approved these changes Jan 5, 2026

View reviewed changes

		@@ -1,3 +1,11 @@
		import React from 'react';
		import { Post } from '../PostInfo/Post';

		@@ -1,3 +1,14 @@
		import React from 'react';
		import { User } from './User';

Uh oh!

Conversation

MateuszSeler commented Jan 4, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants