Skip to content

Class components#1

Open
DenHelloWorld wants to merge 22 commits into
mainfrom
class-components
Open

Class components#1
DenHelloWorld wants to merge 22 commits into
mainfrom
class-components

Conversation

@DenHelloWorld
Copy link
Copy Markdown
Owner

@DenHelloWorld DenHelloWorld commented Apr 28, 2026

Pull Request Description

1. Task: React project setup. Class components. Error boundary

2. Screenshot:

image

3. Deployment: N/A

4. Done 28.04.2026 / deadline 05.05.2026

5. Score: ? / 100

Functional Requirements (100 / 100)

  • Feature 1: Application Layout Structure (5/5)
    • Page is organized into two distinct sections: search top-bar and results area.
  • Feature 2: Search Functionality with Local Storage (15/15)
    • Application checks local storage on load and populates the input field if a term exists.
  • Feature 3: Search Results Display (10/10)
    • Each result item displays a name and description in a clear format.
  • Feature 4: Initial Data Load (10/10)
    • Fetches data automatically on mount based on the current search term or retrieves all items.
  • Feature 5: Search Execution (20/20)
    • Trims input, prevents redundant requests for the same term, and updates results on button click.
  • Feature 6: Search Term Persistence (5/5)
    • Updated search terms are correctly saved to local storage.
  • Feature 7: Loading State Indication (10/10)
    • A loading indicator is visible while fetching data and hidden once completed.
  • Feature 8: Error Handling (10/10)
    • Meaningful error messages are displayed for 4xx/5xx API responses without console clutter.
  • Feature 9: Application Error Boundary (15/15)
    • Error Boundary implemented with a fallback UI and a dedicated button to simulate an app crash.

Technical Requirements Check

  • Vite + React-TS: Project setup follows the required template.
  • No Hooks: State and lifecycle methods are handled exclusively via Class Components.
  • TypeScript: No any or @ts-ignore used.
  • No State Managers: Redux or similar libraries are not used.
  • No UI Kits: Component libraries (MUI, AntD, etc.) are not used.
  • Branching: Work is located in the class-components branch.

Implementation Details

  • API used: [Public API The Art Institute of Chicago]
  • Styling approach: [Tailwind]

@DenHelloWorld DenHelloWorld self-assigned this Apr 28, 2026
@DenHelloWorld DenHelloWorld added the enhancement New feature or request label Apr 28, 2026
Copy link
Copy Markdown
Collaborator

@Elena-lucky Elena-lucky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect work! There are some suggestions for improvements, but most of them are minor. Keep it up!

Comment thread eslint.config.js
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your config is modern (flat config + TypeScript + React), which is great)
But it is too lightweight and misses many important rules that are standard in real production projects.

Below is a list of what should be added and why it matters.

  1. Import rules (project structure & maintainability)

You are missing eslint-plugin-import, which is critical.

Add:

import/order
import/no-cycle
import/prefer-default-export (optional)
disable import/extensions

Why:

Prevents circular dependencies
Keeps imports clean and predictable
Enforces consistent module structure
2. Strict TypeScript rules (code safety)

Add:

@typescript-eslint/consistent-type-imports
@typescript-eslint/consistent-type-exports
@typescript-eslint/explicit-function-return-type
@typescript-eslint/no-explicit-any
@typescript-eslint/no-non-null-assertion
@typescript-eslint/no-inferrable-types

Why:

Enforces strong typing discipline
Prevents hidden runtime errors
Makes code easier to refactor and scale
3. Unused variables handling (real-world standard)

Add:

@typescript-eslint/no-unused-vars with _ ignore pattern

Why:

Avoids noise from unused variables
Keeps flexibility for intentionally unused params (e.g. _event)
4. Safety rules (production readiness)

Add:

no-console (allow only warn and error)
no-debugger
curly
no-param-reassign
prefer-const

Why:

Prevents accidental debug code in production
Avoids subtle bugs
Encourages predictable code behavior

  1. Architectural constraints (very important)

Add:

max-lines-per-function

Why:

Prevents overly large components/functions
Encourages better separation of concerns
6. Type-aware linting (critical missing piece)

Update your config:

ecmaVersion: 'latest'
add parserOptions.project: './tsconfig.json'

Why:

Enables full TypeScript type-aware rules
Without this, many rules don’t work correctly

  1. Ignore patterns

Add:

node_modules

Why:

Avoid unnecessary linting of dependencies

These are strict (even "hardcore") rules, especially for a learning project.

You may feel that:

they slow you down
they are too restrictive

That’s normal.

But:

Learning to work with strict rules early = much faster growth as a developer.

Comment thread src/components/ErrorBoundary.tsx Outdated
Comment on lines +25 to +27
componentDidCatch(error: Error) {
console.error(error);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

componentDidCatch should include errorInfo
Please, fix
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { console.error(error, errorInfo.componentStack); }
errorInfo.componentStack is extremely useful for debugging
This is the standard React API usage

Comment thread src/components/ErrorBoundary.tsx Outdated
Comment on lines +21 to +23
static getDerivedStateFromError() {
return { isError: true };
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing explicit return type
Missing error argument

Suggested change
static getDerivedStateFromError() {
return { isError: true };
}
static getDerivedStateFromError(_error: Error): ErrorBoundaryState) {
return { isError: true };
}

Comment thread src/components/ErrorBoundary.tsx Outdated
});
};

#onKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onKeyDown is attached to a div. Should be React.KeyboardEvent.

Comment thread src/components/ErrorBoundary.tsx Outdated
import { KEYBOARD_KEYS } from '../consts/keyboard-keys.const.ts';

export interface ErrorBoundaryState {
isError: boolean;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor improvement. May be, it's better to name hasError. It matches React docs convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants