Skip to content

Latest commit

 

History

History
876 lines (739 loc) · 20.4 KB

code-style.md

File metadata and controls

876 lines (739 loc) · 20.4 KB

📝 Code Style Guide

This document outlines the coding standards and best practices for the OpenLN project. Following these guidelines ensures consistency, readability, and maintainability across the codebase.

🎯 General Principles

1. Readability First

  • Write code that tells a story
  • Use descriptive names for variables, functions, and classes
  • Prefer explicit over implicit
  • Add comments only when necessary to explain "why", not "what"

2. Consistency

  • Follow established patterns in the codebase
  • Use consistent naming conventions
  • Maintain consistent file structure
  • Apply formatting rules uniformly

3. Simplicity

  • Keep functions small and focused
  • Avoid over-engineering
  • Use standard library functions when possible
  • Minimize dependencies

🔧 Tools and Configuration

ESLint Configuration

Frontend (client/eslint.config.js)

import js from '@eslint/js'
import globals from 'globals'
import reactHooks from 'eslint-plugin-react-hooks'
import reactRefresh from 'eslint-plugin-react-refresh'
import tseslint from 'typescript-eslint'

export default tseslint.config(
  { ignores: ['dist'] },
  {
    extends: [js.configs.recommended, ...tseslint.configs.recommended],
    files: ['**/*.{ts,tsx}'],
    languageOptions: {
      ecmaVersion: 2020,
      globals: globals.browser,
    },
    plugins: {
      'react-hooks': reactHooks,
      'react-refresh': reactRefresh,
    },
    rules: {
      ...reactHooks.configs.recommended.rules,
      'react-refresh/only-export-components': [
        'warn',
        { allowConstantExport: true },
      ],
      // Custom rules
      'prefer-const': 'error',
      'no-var': 'error',
      '@typescript-eslint/no-unused-vars': 'error',
      '@typescript-eslint/explicit-function-return-type': 'warn',
      '@typescript-eslint/no-explicit-any': 'warn',
    },
  },
)

Backend (server/.eslintrc.js)

module.exports = {
  env: {
    node: true,
    es2022: true,
  },
  extends: [
    'eslint:recommended',
  ],
  parserOptions: {
    ecmaVersion: 'latest',
    sourceType: 'module',
  },
  rules: {
    'indent': ['error', 2],
    'linebreak-style': ['error', 'unix'],
    'quotes': ['error', 'single'],
    'semi': ['error', 'always'],
    'no-console': 'warn',
    'no-unused-vars': 'error',
    'prefer-const': 'error',
    'no-var': 'error',
  },
};

Prettier Configuration

.prettierrc

{
  "semi": true,
  "trailingComma": "es5",
  "singleQuote": true,
  "printWidth": 80,
  "tabWidth": 2,
  "useTabs": false,
  "bracketSpacing": true,
  "arrowParens": "avoid",
  "endOfLine": "lf"
}

📱 Frontend Code Style

TypeScript/React Guidelines

Component Structure

// ✅ Good: Clear component structure
import React, { useState, useEffect } from 'react';
import { useNavigate } from 'react-router-dom';

import { Button } from '../common/Button';
import { useAuth } from '../../hooks/useAuth';
import type { User } from '../../types/user';

interface UserProfileProps {
  userId: string;
  onUserUpdate?: (user: User) => void;
}

export const UserProfile: React.FC<UserProfileProps> = ({
  userId,
  onUserUpdate,
}) => {
  const [user, setUser] = useState<User | null>(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState<string | null>(null);
  
  const { currentUser } = useAuth();
  const navigate = useNavigate();

  useEffect(() => {
    const fetchUser = async (): Promise<void> => {
      try {
        setLoading(true);
        const userData = await getUserById(userId);
        setUser(userData);
      } catch (err) {
        setError(err instanceof Error ? err.message : 'Unknown error');
      } finally {
        setLoading(false);
      }
    };

    fetchUser();
  }, [userId]);

  const handleUpdateProfile = async (data: Partial<User>): Promise<void> => {
    try {
      const updatedUser = await updateUser(userId, data);
      setUser(updatedUser);
      onUserUpdate?.(updatedUser);
    } catch (err) {
      setError(err instanceof Error ? err.message : 'Update failed');
    }
  };

  if (loading) return <LoadingSpinner />;
  if (error) return <ErrorMessage message={error} />;
  if (!user) return <NotFoundMessage />;

  return (
    <div className="user-profile">
      <h1>{user.name}</h1>
      <p>{user.email}</p>
      {currentUser?.id === userId && (
        <Button onClick={() => navigate('/profile/edit')}>
          Edit Profile
        </Button>
      )}
    </div>
  );
};

Naming Conventions

// ✅ Good: Descriptive names
const [isSubmitting, setIsSubmitting] = useState(false);
const [validationErrors, setValidationErrors] = useState<string[]>([]);
const [userProfileData, setUserProfileData] = useState<User | null>(null);

// ❌ Bad: Unclear names
const [loading, setLoading] = useState(false);
const [errors, setErrors] = useState([]);
const [data, setData] = useState(null);

Custom Hooks

// ✅ Good: Well-structured custom hook
export const useApi = <T>(url: string) => {
  const [data, setData] = useState<T | null>(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState<string | null>(null);

  useEffect(() => {
    const fetchData = async (): Promise<void> => {
      try {
        setLoading(true);
        setError(null);
        const response = await fetch(url);
        
        if (!response.ok) {
          throw new Error(`HTTP error! status: ${response.status}`);
        }
        
        const result = await response.json();
        setData(result);
      } catch (err) {
        setError(err instanceof Error ? err.message : 'An error occurred');
      } finally {
        setLoading(false);
      }
    };

    fetchData();
  }, [url]);

  return { data, loading, error };
};

Type Definitions

// ✅ Good: Comprehensive type definitions
export interface User {
  readonly id: string;
  email: string;
  name: string;
  profile: UserProfile;
  preferences: UserPreferences;
  createdAt: Date;
  updatedAt: Date;
}

export interface UserProfile {
  avatar?: string;
  bio?: string;
  location?: string;
  website?: string;
}

export interface UserPreferences {
  theme: 'light' | 'dark' | 'system';
  notifications: {
    email: boolean;
    push: boolean;
    marketing: boolean;
  };
  learningStyle: 'visual' | 'auditory' | 'kinesthetic';
}

// API response types
export interface ApiResponse<T> {
  success: boolean;
  data: T;
  message?: string;
  errors?: string[];
}

export interface PaginatedResponse<T> extends ApiResponse<T> {
  pagination: {
    currentPage: number;
    totalPages: number;
    totalItems: number;
    itemsPerPage: number;
  };
}

CSS and Styling

TailwindCSS Guidelines

// ✅ Good: Organized Tailwind classes
<div className="
  flex flex-col items-center justify-center
  min-h-screen p-4
  bg-gray-50 dark:bg-gray-900
">
  <div className="
    w-full max-w-md p-6
    bg-white dark:bg-gray-800
    rounded-lg shadow-lg
    border border-gray-200 dark:border-gray-700
  ">
    <h1 className="
      text-2xl font-bold text-center
      text-gray-900 dark:text-white
      mb-6
    ">
      Login
    </h1>
  </div>
</div>

// ❌ Bad: Long, unorganized class strings
<div className="flex flex-col items-center justify-center min-h-screen p-4 bg-gray-50 dark:bg-gray-900">

Component-Specific Styles

/* UserProfile.module.css */
.userProfile {
  @apply max-w-4xl mx-auto p-6;
}

.userProfile__header {
  @apply flex items-center gap-4 mb-6;
}

.userProfile__avatar {
  @apply w-16 h-16 rounded-full object-cover;
}

.userProfile__name {
  @apply text-2xl font-bold text-gray-900 dark:text-white;
}

.userProfile__email {
  @apply text-gray-600 dark:text-gray-300;
}

⚙️ Backend Code Style

Node.js/Express Guidelines

Controller Structure

// ✅ Good: Well-structured controller
import { validationResult } from 'express-validator';
import { userService } from '../services/userService.js';
import { logger } from '../utils/logger.js';

export const getUserById = async (req, res, next) => {
  try {
    const { id } = req.params;
    
    // Validate input
    const errors = validationResult(req);
    if (!errors.isEmpty()) {
      return res.status(400).json({
        success: false,
        errors: errors.array(),
      });
    }

    // Business logic
    const user = await userService.findById(id);
    
    if (!user) {
      return res.status(404).json({
        success: false,
        message: 'User not found',
      });
    }

    // Success response
    res.status(200).json({
      success: true,
      data: user,
    });
  } catch (error) {
    logger.error('Error fetching user:', error);
    next(error);
  }
};

export const createUser = async (req, res, next) => {
  try {
    const userData = req.body;
    
    // Validate input
    const errors = validationResult(req);
    if (!errors.isEmpty()) {
      return res.status(400).json({
        success: false,
        errors: errors.array(),
      });
    }

    // Create user
    const newUser = await userService.create(userData);
    
    logger.info(`User created successfully: ${newUser.id}`);
    
    res.status(201).json({
      success: true,
      data: newUser,
      message: 'User created successfully',
    });
  } catch (error) {
    if (error.code === 11000) {
      return res.status(409).json({
        success: false,
        message: 'User already exists',
      });
    }
    
    logger.error('Error creating user:', error);
    next(error);
  }
};

Service Layer

// ✅ Good: Clean service layer
import { User } from '../models/User.js';
import { hashPassword, comparePassword } from '../utils/encryption.js';
import { logger } from '../utils/logger.js';

class UserService {
  async findById(id) {
    try {
      const user = await User.findById(id).select('-password');
      return user;
    } catch (error) {
      logger.error(`Error finding user by ID ${id}:`, error);
      throw error;
    }
  }

  async create(userData) {
    try {
      const { email, password, ...otherData } = userData;
      
      // Check if user exists
      const existingUser = await User.findOne({ email });
      if (existingUser) {
        const error = new Error('User already exists');
        error.code = 11000;
        throw error;
      }

      // Hash password
      const hashedPassword = await hashPassword(password);
      
      // Create user
      const user = new User({
        email,
        password: hashedPassword,
        ...otherData,
      });

      await user.save();
      
      // Return user without password
      return user.toObject({ transform: removePassword });
    } catch (error) {
      logger.error('Error creating user:', error);
      throw error;
    }
  }

  async updateById(id, updateData) {
    try {
      const user = await User.findByIdAndUpdate(
        id,
        { $set: updateData },
        { new: true, runValidators: true }
      ).select('-password');

      return user;
    } catch (error) {
      logger.error(`Error updating user ${id}:`, error);
      throw error;
    }
  }

  async deleteById(id) {
    try {
      const result = await User.findByIdAndDelete(id);
      return result;
    } catch (error) {
      logger.error(`Error deleting user ${id}:`, error);
      throw error;
    }
  }
}

// Helper function
const removePassword = (doc, ret) => {
  delete ret.password;
  return ret;
};

export const userService = new UserService();

Model Definitions

// ✅ Good: Well-defined Mongoose model
import mongoose from 'mongoose';
import validator from 'validator';

const userSchema = new mongoose.Schema({
  email: {
    type: String,
    required: [true, 'Email is required'],
    unique: true,
    lowercase: true,
    validate: [validator.isEmail, 'Please provide a valid email'],
  },
  
  password: {
    type: String,
    required: [true, 'Password is required'],
    minlength: [8, 'Password must be at least 8 characters long'],
  },
  
  name: {
    type: String,
    required: [true, 'Name is required'],
    trim: true,
    maxlength: [100, 'Name cannot exceed 100 characters'],
  },
  
  profile: {
    avatar: {
      type: String,
      validate: [validator.isURL, 'Avatar must be a valid URL'],
    },
    bio: {
      type: String,
      maxlength: [500, 'Bio cannot exceed 500 characters'],
    },
    location: {
      type: String,
      maxlength: [100, 'Location cannot exceed 100 characters'],
    },
  },
  
  preferences: {
    theme: {
      type: String,
      enum: ['light', 'dark', 'system'],
      default: 'system',
    },
    notifications: {
      email: { type: Boolean, default: true },
      push: { type: Boolean, default: false },
    },
    learningStyle: {
      type: String,
      enum: ['visual', 'auditory', 'kinesthetic'],
    },
  },
  
  role: {
    type: String,
    enum: ['user', 'instructor', 'admin'],
    default: 'user',
  },
  
  isActive: {
    type: Boolean,
    default: true,
  },
}, {
  timestamps: true,
  toJSON: { virtuals: true },
  toObject: { virtuals: true },
});

// Indexes
userSchema.index({ email: 1 });
userSchema.index({ 'profile.name': 'text' });

// Virtual fields
userSchema.virtual('fullProfile').get(function() {
  return {
    id: this._id,
    name: this.name,
    email: this.email,
    avatar: this.profile?.avatar,
    role: this.role,
  };
});

// Instance methods
userSchema.methods.toProfileJSON = function() {
  return {
    id: this._id,
    name: this.name,
    email: this.email,
    profile: this.profile,
    preferences: this.preferences,
    role: this.role,
    createdAt: this.createdAt,
    updatedAt: this.updatedAt,
  };
};

// Static methods
userSchema.statics.findByEmail = function(email) {
  return this.findOne({ email: email.toLowerCase() });
};

export const User = mongoose.model('User', userSchema);

Error Handling

// ✅ Good: Comprehensive error handling middleware
export const errorHandler = (err, req, res, next) => {
  let error = { ...err };
  error.message = err.message;

  // Log error
  logger.error(err);

  // Mongoose bad ObjectId
  if (err.name === 'CastError') {
    const message = 'Resource not found';
    error = { message, statusCode: 404 };
  }

  // Mongoose duplicate key
  if (err.code === 11000) {
    const message = 'Duplicate field value entered';
    error = { message, statusCode: 400 };
  }

  // Mongoose validation error
  if (err.name === 'ValidationError') {
    const message = Object.values(err.errors).map(val => val.message).join(', ');
    error = { message, statusCode: 400 };
  }

  // JWT errors
  if (err.name === 'JsonWebTokenError') {
    const message = 'Invalid token';
    error = { message, statusCode: 401 };
  }

  if (err.name === 'TokenExpiredError') {
    const message = 'Token expired';
    error = { message, statusCode: 401 };
  }

  res.status(error.statusCode || 500).json({
    success: false,
    error: {
      message: error.message || 'Server Error',
      ...(process.env.NODE_ENV === 'development' && { stack: err.stack }),
    },
  });
};

📏 Naming Conventions

Variables and Functions

// ✅ Good: Descriptive names
const userProfileData = getUserProfile();
const isUserAuthenticated = checkAuthStatus();
const calculateTotalLearningTime = (sessions) => { /* ... */ };

// ❌ Bad: Unclear names
const data = getData();
const flag = check();
const calc = (arr) => { /* ... */ };

Files and Directories

// ✅ Good: Clear structure
components/
├── common/
│   ├── Button/
│   │   ├── Button.tsx
│   │   ├── Button.test.tsx
│   │   └── index.ts
│   └── Modal/
└── forms/
    └── LoginForm/

// ❌ Bad: Unclear structure
components/
├── btn.tsx
├── modal.tsx
└── form.tsx

Constants

// ✅ Good: Clear constants
export const API_ENDPOINTS = {
  USERS: '/api/v1/users',
  COURSES: '/api/v1/courses',
  AUTH: '/api/v1/auth',
} as const;

export const HTTP_STATUS_CODES = {
  OK: 200,
  CREATED: 201,
  BAD_REQUEST: 400,
  UNAUTHORIZED: 401,
  NOT_FOUND: 404,
  INTERNAL_SERVER_ERROR: 500,
} as const;

export const VALIDATION_RULES = {
  EMAIL_MAX_LENGTH: 255,
  PASSWORD_MIN_LENGTH: 8,
  NAME_MAX_LENGTH: 100,
} as const;

🧪 Testing Standards

Frontend Tests

// ✅ Good: Comprehensive component test
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { vi } from 'vitest';
import { UserProfile } from './UserProfile';
import * as userService from '../../services/userService';

// Mock the service
vi.mock('../../services/userService');

describe('UserProfile', () => {
  const mockUser = {
    id: '1',
    name: 'John Doe',
    email: 'john@example.com',
    profile: { avatar: 'avatar.jpg' },
  };

  beforeEach(() => {
    vi.clearAllMocks();
  });

  it('should render user information correctly', async () => {
    vi.mocked(userService.getUserById).mockResolvedValue(mockUser);

    render(<UserProfile userId="1" />);

    await waitFor(() => {
      expect(screen.getByText('John Doe')).toBeInTheDocument();
      expect(screen.getByText('john@example.com')).toBeInTheDocument();
    });
  });

  it('should handle loading state', () => {
    vi.mocked(userService.getUserById).mockImplementation(
      () => new Promise(() => {}) // Never resolves
    );

    render(<UserProfile userId="1" />);

    expect(screen.getByText('Loading...')).toBeInTheDocument();
  });

  it('should handle error state', async () => {
    vi.mocked(userService.getUserById).mockRejectedValue(
      new Error('User not found')
    );

    render(<UserProfile userId="1" />);

    await waitFor(() => {
      expect(screen.getByText('User not found')).toBeInTheDocument();
    });
  });

  it('should call onUserUpdate when profile is updated', async () => {
    const mockOnUpdate = vi.fn();
    vi.mocked(userService.getUserById).mockResolvedValue(mockUser);

    render(<UserProfile userId="1" onUserUpdate={mockOnUpdate} />);

    // Test update functionality
    // ... implementation details
  });
});

Backend Tests

// ✅ Good: API endpoint test
import request from 'supertest';
import app from '../app.js';
import { User } from '../models/User.js';

describe('GET /api/v1/users/:id', () => {
  beforeEach(async () => {
    await User.deleteMany({});
  });

  afterAll(async () => {
    await User.deleteMany({});
  });

  it('should return user when valid ID is provided', async () => {
    const user = await User.create({
      name: 'Test User',
      email: 'test@example.com',
      password: 'hashedpassword',
    });

    const response = await request(app)
      .get(`/api/v1/users/${user._id}`)
      .expect(200);

    expect(response.body.success).toBe(true);
    expect(response.body.data.name).toBe('Test User');
    expect(response.body.data.email).toBe('test@example.com');
    expect(response.body.data.password).toBeUndefined();
  });

  it('should return 404 when user not found', async () => {
    const nonExistentId = new mongoose.Types.ObjectId();

    const response = await request(app)
      .get(`/api/v1/users/${nonExistentId}`)
      .expect(404);

    expect(response.body.success).toBe(false);
    expect(response.body.message).toBe('User not found');
  });

  it('should return 400 for invalid ID format', async () => {
    const response = await request(app)
      .get('/api/v1/users/invalid-id')
      .expect(400);

    expect(response.body.success).toBe(false);
  });
});

📋 Code Review Checklist

✅ Before Submitting PR

  • Code follows style guidelines
  • All tests pass
  • TypeScript types are properly defined
  • Error handling is implemented
  • Logging is appropriate
  • No console.log statements in production code
  • Environment variables are used for configuration
  • Security best practices are followed
  • Performance considerations are addressed
  • Documentation is updated if needed

✅ Code Review Points

  • Functionality: Does the code work as intended?
  • Readability: Is the code easy to understand?
  • Performance: Are there any performance issues?
  • Security: Are there any security vulnerabilities?
  • Testing: Is the code properly tested?
  • Documentation: Are complex parts documented?
  • Consistency: Does the code follow project conventions?
  • Error Handling: Are errors handled appropriately?

Following these code style guidelines helps maintain a high-quality, consistent codebase that's easy to understand, maintain, and extend. When in doubt, refer to existing code patterns in the project or ask for clarification in code reviews.