Fixes/request deduplication #44
@@ -1,592 +0,0 @@
|
|||||||
# Git Workflow and Branching Strategy
|
|
||||||
|
|
||||||
This document outlines the git workflow, branching strategy, commit conventions, and code review guidelines for the glyphdiff.com project.
|
|
||||||
|
|
||||||
## Table of Contents
|
|
||||||
|
|
||||||
1. [Branching Strategy](#branching-strategy)
|
|
||||||
2. [Branch Naming Conventions](#branch-naming-conventions)
|
|
||||||
3. [Commit Message Conventions](#commit-message-conventions)
|
|
||||||
4. [Code Splitting and Merge Request Guidelines](#code-splitting-and-merge-request-guidelines)
|
|
||||||
5. [Branch Protection Rules](#branch-protection-rules)
|
|
||||||
6. [Git Hooks Configuration](#git-hooks-configuration)
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Branching Strategy
|
|
||||||
|
|
||||||
We use a Gitflow-inspired branching strategy adapted for our development workflow. This strategy provides a clear structure for feature development, bug fixes, and releases.
|
|
||||||
|
|
||||||
### Branch Types
|
|
||||||
|
|
||||||
#### 1. `main` Branch
|
|
||||||
- **Purpose**: Production-ready code only
|
|
||||||
- **Protection**: Highest level of protection
|
|
||||||
- **Rules**:
|
|
||||||
- Only merge `release/*` or `hotfix/*` branches into `main`
|
|
||||||
- No direct commits allowed
|
|
||||||
- Must pass all tests and code reviews
|
|
||||||
- Tags are created from this branch for releases (e.g., `v1.0.0`)
|
|
||||||
|
|
||||||
#### 2. `develop` Branch
|
|
||||||
- **Purpose**: Integration branch for features
|
|
||||||
- **Protection**: High level of protection
|
|
||||||
- **Rules**:
|
|
||||||
- Merge `feature/*` and `fix/*` branches into `develop`
|
|
||||||
- No direct commits allowed
|
|
||||||
- Must pass all tests before merging
|
|
||||||
- Serves as the base for `release/*` branches
|
|
||||||
|
|
||||||
#### 3. `feature/*` Branches
|
|
||||||
- **Purpose**: Develop new features
|
|
||||||
- **Naming**: `feature/feature-name` (e.g., `feature/font-catalog`, `feature/comparison-grid`)
|
|
||||||
- **Base**: Always branch from `develop`
|
|
||||||
- **Merge**: Merge back into `develop` via Merge Request (MR)
|
|
||||||
- **Rules**:
|
|
||||||
- One feature per branch
|
|
||||||
- Keep branches focused and small
|
|
||||||
- Delete after merging
|
|
||||||
|
|
||||||
#### 4. `fix/*` Branches
|
|
||||||
- **Purpose**: Fix bugs discovered during development
|
|
||||||
- **Naming**: `fix/issue-description` (e.g., `fix/font-loading-error`, `fix/responsive-layout`)
|
|
||||||
- **Base**: Branch from `develop`
|
|
||||||
- **Merge**: Merge back into `develop` via MR
|
|
||||||
- **Rules**:
|
|
||||||
- One fix per branch
|
|
||||||
- Include tests that verify the fix
|
|
||||||
- Delete after merging
|
|
||||||
|
|
||||||
#### 5. `hotfix/*` Branches
|
|
||||||
- **Purpose**: Critical fixes for production issues
|
|
||||||
- **Naming**: `hotfix/critical-fix` (e.g., `hotfix/security-patch`, `hotfix-production-crash`)
|
|
||||||
- **Base**: Branch from `main`
|
|
||||||
- **Merge**: Merge into both `main` and `develop`
|
|
||||||
- **Rules**:
|
|
||||||
- Use only for production emergencies
|
|
||||||
- Must be thoroughly tested
|
|
||||||
- Create a release tag after merging to `main`
|
|
||||||
|
|
||||||
#### 6. `release/*` Branches
|
|
||||||
- **Purpose**: Prepare for a new release
|
|
||||||
- **Naming**: `release/vX.Y.Z` (e.g., `release/v1.0.0`, `release/v1.1.0`)
|
|
||||||
- **Base**: Branch from `develop`
|
|
||||||
- **Merge**: Merge into both `main` and `develop`
|
|
||||||
- **Rules**:
|
|
||||||
- Finalize release notes
|
|
||||||
- Update version numbers
|
|
||||||
- Perform final testing
|
|
||||||
- Create release tag after merging to `main`
|
|
||||||
|
|
||||||
### Branch Workflow Diagram
|
|
||||||
|
|
||||||
```
|
|
||||||
main (production)
|
|
||||||
↑
|
|
||||||
│ hotfix/*, release/*
|
|
||||||
│
|
|
||||||
develop (integration)
|
|
||||||
↑
|
|
||||||
│ feature/*, fix/*
|
|
||||||
│
|
|
||||||
feature branches
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Branch Naming Conventions
|
|
||||||
|
|
||||||
### Feature Branches
|
|
||||||
- Format: `feature/feature-name`
|
|
||||||
- Examples:
|
|
||||||
- `feature/font-catalog`
|
|
||||||
- `feature/comparison-grid`
|
|
||||||
- `feature/dark-mode`
|
|
||||||
- `feature/google-fonts-integration`
|
|
||||||
|
|
||||||
### Fix Branches
|
|
||||||
- Format: `fix/issue-description`
|
|
||||||
- Examples:
|
|
||||||
- `fix/font-loading-error`
|
|
||||||
- `fix/responsive-layout`
|
|
||||||
- `fix/state-persistence`
|
|
||||||
- `fix-accessibility-contrast`
|
|
||||||
|
|
||||||
### Hotfix Branches
|
|
||||||
- Format: `hotfix/critical-fix`
|
|
||||||
- Examples:
|
|
||||||
- `hotfix/security-patch`
|
|
||||||
- `hotfix-production-crash`
|
|
||||||
- `hotfix-api-rate-limit`
|
|
||||||
|
|
||||||
### Release Branches
|
|
||||||
- Format: `release/vX.Y.Z`
|
|
||||||
- Examples:
|
|
||||||
- `release/v1.0.0`
|
|
||||||
- `release/v1.1.0`
|
|
||||||
- `release/v2.0.0`
|
|
||||||
|
|
||||||
### Naming Guidelines
|
|
||||||
- Use lowercase letters
|
|
||||||
- Use hyphens to separate words
|
|
||||||
- Be descriptive but concise
|
|
||||||
- Avoid special characters (except hyphens)
|
|
||||||
- Keep names under 50 characters
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Commit Message Conventions
|
|
||||||
|
|
||||||
We follow the [Conventional Commits](https://www.conventionalcommits.org/) specification. This format enables automated changelog generation and better commit history readability.
|
|
||||||
|
|
||||||
### Format
|
|
||||||
|
|
||||||
```
|
|
||||||
<type>(<scope>): <subject>
|
|
||||||
|
|
||||||
<body>
|
|
||||||
|
|
||||||
<footer>
|
|
||||||
```
|
|
||||||
|
|
||||||
### Commit Types
|
|
||||||
|
|
||||||
| Type | Description | Examples |
|
|
||||||
|------|-------------|----------|
|
|
||||||
| `feat` | New feature | `feat(fonts): add Google Fonts integration` |
|
|
||||||
| `fix` | Bug fix | `fix(comparison): resolve font loading race condition` |
|
|
||||||
| `docs` | Documentation changes | `docs(readme): update installation instructions` |
|
|
||||||
| `style` | Code style changes (formatting, etc.) | `style(components): format with Prettier` |
|
|
||||||
| `refactor` | Code refactoring | `refactor(stores): simplify state management` |
|
|
||||||
| `test` | Adding or updating tests | `test(fonts): add unit tests for font mapper` |
|
|
||||||
| `chore` | Maintenance tasks | `chore(deps): update Tailwind CSS to v4.0` |
|
|
||||||
| `perf` | Performance improvements | `perf(catalog): implement lazy loading for fonts` |
|
|
||||||
|
|
||||||
### Scope
|
|
||||||
|
|
||||||
The scope provides context about which part of the codebase is affected. Common scopes for this project:
|
|
||||||
|
|
||||||
- `fonts` - Font-related functionality
|
|
||||||
- `comparison` - Font comparison features
|
|
||||||
- `catalog` - Font catalog pages
|
|
||||||
- `stores` - State management stores
|
|
||||||
- `components` - UI components
|
|
||||||
- `routes` - SvelteKit routes
|
|
||||||
- `services` - External API services
|
|
||||||
- `utils` - Utility functions
|
|
||||||
- `types` - TypeScript type definitions
|
|
||||||
- `ui` - UI-related changes (theme, layout, etc.)
|
|
||||||
- `config` - Configuration files
|
|
||||||
|
|
||||||
### Subject
|
|
||||||
|
|
||||||
- Use imperative mood ("add" not "added", "fix" not "fixed")
|
|
||||||
- Keep it short (50 characters or less)
|
|
||||||
- Don't end with a period
|
|
||||||
- Be specific and descriptive
|
|
||||||
|
|
||||||
### Body
|
|
||||||
|
|
||||||
- Use imperative mood
|
|
||||||
- Explain **what** and **why**, not **how**
|
|
||||||
- Wrap at 72 characters
|
|
||||||
- Include references to issues (e.g., `Closes #123`)
|
|
||||||
|
|
||||||
### Footer
|
|
||||||
|
|
||||||
- Reference breaking changes with `BREAKING CHANGE:`
|
|
||||||
- Reference issues with `Closes #123` or `Fixes #456`
|
|
||||||
- Include co-authors if needed
|
|
||||||
|
|
||||||
### Examples
|
|
||||||
|
|
||||||
#### Feature Commit
|
|
||||||
```
|
|
||||||
feat(fonts): add Google Fonts API integration
|
|
||||||
|
|
||||||
Implement Google Fonts API service to fetch and display available fonts.
|
|
||||||
This includes the fetchGoogleFonts function and font mapper utilities.
|
|
||||||
|
|
||||||
Closes #12
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Bug Fix Commit
|
|
||||||
```
|
|
||||||
fix(comparison): resolve font loading race condition
|
|
||||||
|
|
||||||
The comparison grid was attempting to render fonts before they were fully
|
|
||||||
loaded. Added loading state checks to prevent this issue.
|
|
||||||
|
|
||||||
Fixes #45
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Refactor Commit
|
|
||||||
```
|
|
||||||
refactor(stores): simplify state management with Svelte 5 runes
|
|
||||||
|
|
||||||
Migrated from Svelte stores to Svelte 5's $state runes for better
|
|
||||||
performance and simpler code. This change affects all stores in the
|
|
||||||
project.
|
|
||||||
|
|
||||||
BREAKING CHANGE: Store API has changed from subscribe() to direct
|
|
||||||
property access. Update all store consumers accordingly.
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Documentation Commit
|
|
||||||
```
|
|
||||||
docs(git-workflow): add commit message conventions
|
|
||||||
|
|
||||||
Document the conventional commits format with examples and guidelines
|
|
||||||
for the team.
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Chore Commit
|
|
||||||
```
|
|
||||||
chore(deps): update Tailwind CSS to v4.0.0
|
|
||||||
|
|
||||||
Update Tailwind CSS to the latest version and adjust configuration
|
|
||||||
files accordingly.
|
|
||||||
```
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Code Splitting and Merge Request Guidelines
|
|
||||||
|
|
||||||
### Merge Request Size Guidelines
|
|
||||||
|
|
||||||
- **Maximum MR size**: < 500 lines changed (additions + deletions)
|
|
||||||
- **Ideal MR size**: 100-300 lines changed
|
|
||||||
- **Files per MR**: < 10 files
|
|
||||||
|
|
||||||
### When to Split a Feature into Multiple MRs
|
|
||||||
|
|
||||||
Split a feature into multiple MRs when:
|
|
||||||
|
|
||||||
1. **The feature is large** (> 500 lines or > 10 files)
|
|
||||||
2. **Multiple concerns are involved** (e.g., UI + API + state management)
|
|
||||||
3. **Independent parts can be tested separately**
|
|
||||||
4. **The feature has logical phases** (e.g., setup → implementation → polish)
|
|
||||||
|
|
||||||
### Example: Splitting a Feature
|
|
||||||
|
|
||||||
**Feature**: Font Catalog with Filtering
|
|
||||||
|
|
||||||
**MR 1**: `feature/font-catalog-setup`
|
|
||||||
- Create basic catalog page structure
|
|
||||||
- Set up routing
|
|
||||||
- Add placeholder components
|
|
||||||
- ~150 lines
|
|
||||||
|
|
||||||
**MR 2**: `feature/font-catalog-data`
|
|
||||||
- Implement Google Fonts API integration
|
|
||||||
- Create font data fetching logic
|
|
||||||
- Add font mapper utilities
|
|
||||||
- ~200 lines
|
|
||||||
|
|
||||||
**MR 3**: `feature/font-catalog-ui`
|
|
||||||
- Build FontCard component
|
|
||||||
- Implement grid layout
|
|
||||||
- Add loading states
|
|
||||||
- ~250 lines
|
|
||||||
|
|
||||||
**MR 4**: `feature/font-catalog-filtering`
|
|
||||||
- Implement filter store
|
|
||||||
- Build FilterBar component
|
|
||||||
- Connect filters to catalog
|
|
||||||
- ~180 lines
|
|
||||||
|
|
||||||
### Merge Request Description Template
|
|
||||||
|
|
||||||
Every MR must include a comprehensive description:
|
|
||||||
|
|
||||||
```markdown
|
|
||||||
## Description
|
|
||||||
Brief description of what this MR changes and why.
|
|
||||||
|
|
||||||
## Changes Made
|
|
||||||
- [ ] Change 1
|
|
||||||
- [ ] Change 2
|
|
||||||
- [ ] Change 3
|
|
||||||
|
|
||||||
## Type of Change
|
|
||||||
- [ ] Bug fix
|
|
||||||
- [ ] New feature
|
|
||||||
- [ ] Breaking change
|
|
||||||
- [ ] Documentation update
|
|
||||||
- [ ] Refactoring
|
|
||||||
- [ ] Performance improvement
|
|
||||||
|
|
||||||
## Testing
|
|
||||||
- [ ] Unit tests pass
|
|
||||||
- [ ] Manual testing completed
|
|
||||||
- [ ] Tested on Chrome
|
|
||||||
- [ ] Tested on Firefox
|
|
||||||
- [ ] Tested on Safari
|
|
||||||
- [ ] Tested on mobile (responsive)
|
|
||||||
|
|
||||||
## Screenshots (if applicable)
|
|
||||||
Add screenshots or GIFs showing the changes.
|
|
||||||
|
|
||||||
## Checklist
|
|
||||||
- [ ] Code follows project style guidelines
|
|
||||||
- [ ] Self-review completed
|
|
||||||
- [ ] Comments added for complex logic
|
|
||||||
- [ ] Documentation updated
|
|
||||||
- [ ] No new warnings generated
|
|
||||||
- [ ] Tests added/updated
|
|
||||||
- [ ] All tests passing
|
|
||||||
|
|
||||||
## Related Issues
|
|
||||||
Closes #123
|
|
||||||
Related to #456
|
|
||||||
```
|
|
||||||
|
|
||||||
### Code Review Checklist
|
|
||||||
|
|
||||||
Reviewers should check:
|
|
||||||
|
|
||||||
#### Functionality
|
|
||||||
- [ ] Does the code work as intended?
|
|
||||||
- [ ] Are edge cases handled?
|
|
||||||
- [ ] Is error handling appropriate?
|
|
||||||
|
|
||||||
#### Code Quality
|
|
||||||
- [ ] Is the code readable and maintainable?
|
|
||||||
- [ ] Are variable/function names descriptive?
|
|
||||||
- [ ] Is there unnecessary complexity?
|
|
||||||
- [ ] Are there code duplications?
|
|
||||||
|
|
||||||
#### Best Practices
|
|
||||||
- [ ] Does it follow project conventions?
|
|
||||||
- [ ] Are TypeScript types properly defined?
|
|
||||||
- [ ] Are Svelte best practices followed?
|
|
||||||
- [ ] Is Tailwind CSS used appropriately?
|
|
||||||
|
|
||||||
#### Testing
|
|
||||||
- [ ] Are tests included?
|
|
||||||
- [ ] Do tests cover edge cases?
|
|
||||||
- [ ] Are tests meaningful and not redundant?
|
|
||||||
|
|
||||||
#### Documentation
|
|
||||||
- [ ] Is the code self-documenting?
|
|
||||||
- [ ] Are complex functions commented?
|
|
||||||
- [ ] Is the MR description clear?
|
|
||||||
|
|
||||||
#### Performance
|
|
||||||
- [ ] Are there performance concerns?
|
|
||||||
- [ ] Is lazy loading used where appropriate?
|
|
||||||
- [ ] Are unnecessary re-renders avoided?
|
|
||||||
|
|
||||||
### Merge Request Approval Process
|
|
||||||
|
|
||||||
1. **Author**: Creates MR with complete description
|
|
||||||
2. **Reviewer**: Reviews code using the checklist above
|
|
||||||
3. **Discussion**: Address any concerns or suggestions
|
|
||||||
4. **Approval**: At least one approval required
|
|
||||||
4. **Merge**: Squash and merge into target branch
|
|
||||||
5. **Cleanup**: Delete source branch after merge
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Branch Protection Rules
|
|
||||||
|
|
||||||
### `main` Branch Protection
|
|
||||||
|
|
||||||
- **Require pull request reviews**: Yes
|
|
||||||
- Required approvers: 1
|
|
||||||
- Dismiss stale reviews: Yes
|
|
||||||
- **Require status checks**: Yes
|
|
||||||
- Required checks: All tests, linting
|
|
||||||
- Require branches to be up to date: Yes
|
|
||||||
- **Restrict who can push**: Only maintainers
|
|
||||||
- **Require linear history**: Yes (squash and merge)
|
|
||||||
- **Block force pushes**: Yes
|
|
||||||
|
|
||||||
### `develop` Branch Protection
|
|
||||||
|
|
||||||
- **Require pull request reviews**: Yes
|
|
||||||
- Required approvers: 1
|
|
||||||
- Dismiss stale reviews: Yes
|
|
||||||
- **Require status checks**: Yes
|
|
||||||
- Required checks: All tests, linting
|
|
||||||
- Require branches to be up to date: Yes
|
|
||||||
- **Restrict who can push**: Only developers and maintainers
|
|
||||||
- **Require linear history**: Yes (squash and merge)
|
|
||||||
- **Block force pushes**: Yes
|
|
||||||
|
|
||||||
### Implementation Notes
|
|
||||||
|
|
||||||
These rules should be configured in your Git hosting platform (GitHub, GitLab, or Bitbucket). The exact configuration steps vary by platform:
|
|
||||||
|
|
||||||
- **GitHub**: Settings → Branches → Add rule
|
|
||||||
- **GitLab**: Settings → Repository → Protected branches
|
|
||||||
- **Bitbucket**: Repository settings → Branch restrictions
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Git Hooks Configuration
|
|
||||||
|
|
||||||
Git hooks are automated scripts that run at specific points in the git workflow. They help maintain code quality and consistency.
|
|
||||||
|
|
||||||
### Recommended Hooks
|
|
||||||
|
|
||||||
#### 1. Pre-commit Hook
|
|
||||||
**Purpose**: Run linter and formatter before committing
|
|
||||||
|
|
||||||
**Tools**: ESLint, Prettier
|
|
||||||
|
|
||||||
**Implementation**:
|
|
||||||
```bash
|
|
||||||
#!/bin/sh
|
|
||||||
# .git/hooks/pre-commit
|
|
||||||
|
|
||||||
# Run Prettier
|
|
||||||
npm run format:check
|
|
||||||
|
|
||||||
# Run ESLint
|
|
||||||
npm run lint
|
|
||||||
|
|
||||||
# Exit with error if any check fails
|
|
||||||
if [ $? -ne 0 ]; then
|
|
||||||
echo "❌ Pre-commit checks failed. Please fix the issues before committing."
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo "✅ Pre-commit checks passed."
|
|
||||||
```
|
|
||||||
|
|
||||||
**Setup**:
|
|
||||||
```bash
|
|
||||||
# Install husky (recommended)
|
|
||||||
npm install --save-dev husky
|
|
||||||
|
|
||||||
# Initialize husky
|
|
||||||
npx husky install
|
|
||||||
|
|
||||||
# Add pre-commit hook
|
|
||||||
npx husky add .husky/pre-commit "npm run lint && npm run format:check"
|
|
||||||
```
|
|
||||||
|
|
||||||
#### 2. Commit-msg Hook
|
|
||||||
**Purpose**: Validate commit message format
|
|
||||||
|
|
||||||
**Tools**: commitlint
|
|
||||||
|
|
||||||
**Implementation**:
|
|
||||||
```bash
|
|
||||||
#!/bin/sh
|
|
||||||
# .git/hooks/commit-msg
|
|
||||||
|
|
||||||
# Validate commit message with commitlint
|
|
||||||
npx --no -- commitlint --edit $1
|
|
||||||
```
|
|
||||||
|
|
||||||
**Setup**:
|
|
||||||
```bash
|
|
||||||
# Install commitlint
|
|
||||||
npm install --save-dev @commitlint/cli @commitlint/config-conventional
|
|
||||||
|
|
||||||
# Create commitlint config
|
|
||||||
echo "module.exports = { extends: ['@commitlint/config-conventional'] };" > commitlint.config.js
|
|
||||||
|
|
||||||
# Add commit-msg hook
|
|
||||||
npx husky add .husky/commit-msg "npx --no -- commitlint --edit \$1"
|
|
||||||
```
|
|
||||||
|
|
||||||
#### 3. Pre-push Hook
|
|
||||||
**Purpose**: Run tests before pushing
|
|
||||||
|
|
||||||
**Tools**: Vitest, SvelteKit test runner
|
|
||||||
|
|
||||||
**Implementation**:
|
|
||||||
```bash
|
|
||||||
#!/bin/sh
|
|
||||||
# .git/hooks/pre-push
|
|
||||||
|
|
||||||
# Run tests
|
|
||||||
npm run test
|
|
||||||
|
|
||||||
# Exit with error if tests fail
|
|
||||||
if [ $? -ne 0 ]; then
|
|
||||||
echo "❌ Tests failed. Please fix the failing tests before pushing."
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo "✅ All tests passed."
|
|
||||||
```
|
|
||||||
|
|
||||||
**Setup**:
|
|
||||||
```bash
|
|
||||||
# Add pre-push hook
|
|
||||||
npx husky add .husky/pre-push "npm run test"
|
|
||||||
```
|
|
||||||
|
|
||||||
### Alternative: Using Husky
|
|
||||||
|
|
||||||
[Husky](https://typicode.github.io/husky/) is a popular tool for managing git hooks. It's easier to maintain and works across different operating systems.
|
|
||||||
|
|
||||||
**Installation**:
|
|
||||||
```bash
|
|
||||||
npm install --save-dev husky
|
|
||||||
npx husky install
|
|
||||||
npm pkg set scripts.prepare="husky install"
|
|
||||||
```
|
|
||||||
|
|
||||||
**Adding hooks**:
|
|
||||||
```bash
|
|
||||||
# Pre-commit hook
|
|
||||||
npx husky add .husky/pre-commit "npm run lint && npm run format:check"
|
|
||||||
|
|
||||||
# Commit-msg hook
|
|
||||||
npx husky add .husky/commit-msg "npx --no -- commitlint --edit \$1"
|
|
||||||
|
|
||||||
# Pre-push hook
|
|
||||||
npx husky add .husky/pre-push "npm run test"
|
|
||||||
```
|
|
||||||
|
|
||||||
### Hook Scripts for This Project
|
|
||||||
|
|
||||||
Once the project is set up with SvelteKit, add these scripts to `package.json`:
|
|
||||||
|
|
||||||
```json
|
|
||||||
{
|
|
||||||
"scripts": {
|
|
||||||
"format": "prettier --write .",
|
|
||||||
"format:check": "prettier --check .",
|
|
||||||
"lint": "eslint .",
|
|
||||||
"lint:fix": "eslint . --fix",
|
|
||||||
"test": "vitest run",
|
|
||||||
"test:watch": "vitest",
|
|
||||||
"prepare": "husky install"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
### Benefits of Git Hooks
|
|
||||||
|
|
||||||
1. **Consistency**: Enforce code style and formatting
|
|
||||||
2. **Quality**: Catch bugs before they're committed
|
|
||||||
3. **Efficiency**: Fail fast, fix early
|
|
||||||
4. **Automation**: Reduce manual checks
|
|
||||||
5. **Team alignment**: Ensure everyone follows the same standards
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
## Summary
|
|
||||||
|
|
||||||
This git workflow provides a structured approach to development for the glyphdiff.com project:
|
|
||||||
|
|
||||||
- **Clear branching strategy** with defined purposes for each branch type
|
|
||||||
- **Conventional commits** for readable and automated changelogs
|
|
||||||
- **Code splitting guidelines** to keep MRs focused and reviewable
|
|
||||||
- **Comprehensive review process** to maintain code quality
|
|
||||||
- **Git hooks** to automate quality checks
|
|
||||||
|
|
||||||
Following this workflow will help the team:
|
|
||||||
- Develop features in parallel without conflicts
|
|
||||||
- Maintain a clean git history
|
|
||||||
- Catch issues early in the development process
|
|
||||||
- Ensure code quality and consistency
|
|
||||||
- Streamline the release process
|
|
||||||
|
|
||||||
For questions or suggestions about this workflow, please discuss with the team or create an issue in the project repository.
|
|
||||||
@@ -21,6 +21,7 @@ vi.mock('$shared/api/api', () => ({
|
|||||||
import { api } from '$shared/api/api';
|
import { api } from '$shared/api/api';
|
||||||
import { queryClient } from '$shared/api/queryClient';
|
import { queryClient } from '$shared/api/queryClient';
|
||||||
import { fontKeys } from '$shared/api/queryKeys';
|
import { fontKeys } from '$shared/api/queryKeys';
|
||||||
|
import { FontResponseError } from '../../lib/errors/errors';
|
||||||
import {
|
import {
|
||||||
fetchFontsByIds,
|
fetchFontsByIds,
|
||||||
fetchProxyFontById,
|
fetchProxyFontById,
|
||||||
@@ -86,16 +87,20 @@ describe('proxyFonts', () => {
|
|||||||
expect(calledUrl).toContain('offset=0');
|
expect(calledUrl).toContain('offset=0');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('should throw on invalid response (missing fonts array)', async () => {
|
test('should throw FontResponseError on invalid response (missing fonts array)', async () => {
|
||||||
mockApiGet({ total: 0 });
|
mockApiGet({ total: 0 });
|
||||||
|
|
||||||
await expect(fetchProxyFonts()).rejects.toThrow('Proxy API returned invalid response');
|
await expect(fetchProxyFonts()).rejects.toSatisfy(
|
||||||
|
e => e instanceof FontResponseError && e.field === 'response.fonts',
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('should throw on null response data', async () => {
|
test('should throw FontResponseError on null response data', async () => {
|
||||||
vi.mocked(api.get).mockResolvedValueOnce({ data: null, status: 200 });
|
vi.mocked(api.get).mockResolvedValueOnce({ data: null, status: 200 });
|
||||||
|
|
||||||
await expect(fetchProxyFonts()).rejects.toThrow('Proxy API returned invalid response');
|
await expect(fetchProxyFonts()).rejects.toSatisfy(
|
||||||
|
e => e instanceof FontResponseError && e.field === 'response',
|
||||||
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import { queryClient } from '$shared/api/queryClient';
|
|||||||
import { fontKeys } from '$shared/api/queryKeys';
|
import { fontKeys } from '$shared/api/queryKeys';
|
||||||
import { buildQueryString } from '$shared/lib/utils';
|
import { buildQueryString } from '$shared/lib/utils';
|
||||||
import type { QueryParams } from '$shared/lib/utils';
|
import type { QueryParams } from '$shared/lib/utils';
|
||||||
|
import { FontResponseError } from '../../lib/errors/errors';
|
||||||
import type { UnifiedFont } from '../../model/types';
|
import type { UnifiedFont } from '../../model/types';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -96,11 +97,16 @@ export interface ProxyFontsParams extends QueryParams {
|
|||||||
/**
|
/**
|
||||||
* Proxy API response
|
* Proxy API response
|
||||||
*
|
*
|
||||||
* Includes pagination metadata alongside font data
|
* Includes pagination metadata alongside font data.
|
||||||
|
*
|
||||||
|
* Contract: `fonts` is always an array — never `null` or omitted, even when
|
||||||
|
* `total === 0`. Returning `null` on the wire is a backend regression and
|
||||||
|
* surfaces as FontResponseError (non-retryable) on the client.
|
||||||
*/
|
*/
|
||||||
export interface ProxyFontsResponse {
|
export interface ProxyFontsResponse {
|
||||||
/**
|
/**
|
||||||
* List of font objects returned by the proxy
|
* List of font objects returned by the proxy.
|
||||||
|
* Always an array; empty when no matches.
|
||||||
*/
|
*/
|
||||||
fonts: UnifiedFont[];
|
fonts: UnifiedFont[];
|
||||||
|
|
||||||
@@ -156,8 +162,11 @@ export async function fetchProxyFonts(
|
|||||||
|
|
||||||
const response = await api.get<ProxyFontsResponse>(url);
|
const response = await api.get<ProxyFontsResponse>(url);
|
||||||
|
|
||||||
if (!response.data || !Array.isArray(response.data.fonts)) {
|
if (!response.data) {
|
||||||
throw new Error('Proxy API returned invalid response');
|
throw new FontResponseError('response', response.data);
|
||||||
|
}
|
||||||
|
if (!Array.isArray(response.data.fonts)) {
|
||||||
|
throw new FontResponseError('response.fonts', response.data.fonts);
|
||||||
}
|
}
|
||||||
|
|
||||||
return response.data;
|
return response.data;
|
||||||
|
|||||||
@@ -1,3 +1,5 @@
|
|||||||
|
import { NonRetryableError } from '$shared/api/queryClient';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Thrown when the network request to the proxy API fails.
|
* Thrown when the network request to the proxy API fails.
|
||||||
* Wraps the underlying fetch error (timeout, DNS failure, connection refused, etc.).
|
* Wraps the underlying fetch error (timeout, DNS failure, connection refused, etc.).
|
||||||
@@ -12,11 +14,13 @@ export class FontNetworkError extends Error {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Thrown when the proxy API returns a response with an unexpected shape.
|
* Thrown when the proxy API returns a response with an unexpected shape.
|
||||||
|
* Extends NonRetryableError because schema mismatches are not transient —
|
||||||
|
* retrying will produce the same failure and only delay surfacing the bug.
|
||||||
*
|
*
|
||||||
* @property field - The name of the field that failed validation (e.g. `'response'`, `'response.fonts'`).
|
* @property field - The name of the field that failed validation (e.g. `'response'`, `'response.fonts'`).
|
||||||
* @property received - The actual value received at that field, for debugging.
|
* @property received - The actual value received at that field, for debugging.
|
||||||
*/
|
*/
|
||||||
export class FontResponseError extends Error {
|
export class FontResponseError extends NonRetryableError {
|
||||||
readonly name = 'FontResponseError';
|
readonly name = 'FontResponseError';
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
|
|||||||
@@ -84,9 +84,10 @@ describe('FontCatalogStore', () => {
|
|||||||
store.destroy();
|
store.destroy();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('starts with isEmpty false — initial fetch is in progress', () => {
|
it('starts with isEmpty false — observer is gated until setParams enables it', () => {
|
||||||
// The observer starts fetching immediately on construction.
|
// The observer is disabled on construction (no auto-fetch) — see
|
||||||
// isEmpty must be false so the UI shows a loader, not "no results".
|
// `#enabled` in the store. isEmpty must still be false so the UI
|
||||||
|
// doesn't flash "no results" before bindings configures the query.
|
||||||
const store = makeStore();
|
const store = makeStore();
|
||||||
expect(store.isEmpty).toBe(false);
|
expect(store.isEmpty).toBe(false);
|
||||||
store.destroy();
|
store.destroy();
|
||||||
|
|||||||
@@ -31,6 +31,13 @@ type FontStoreResult = InfiniteQueryObserverResult<InfiniteData<ProxyFontsRespon
|
|||||||
|
|
||||||
export class FontCatalogStore {
|
export class FontCatalogStore {
|
||||||
#params = $state<FontStoreParams>({ limit: 50 });
|
#params = $state<FontStoreParams>({ limit: 50 });
|
||||||
|
/**
|
||||||
|
* Gates the initial fetch. The observer starts disabled so the constructor
|
||||||
|
* cannot race ahead of the bindings module — which is the single source of
|
||||||
|
* truth for query params. The first setParams flips this on, producing a
|
||||||
|
* single fetch with the correctly merged queryKey.
|
||||||
|
*/
|
||||||
|
#enabled = $state(false);
|
||||||
#result = $state<FontStoreResult>({} as FontStoreResult);
|
#result = $state<FontStoreResult>({} as FontStoreResult);
|
||||||
#observer: InfiniteQueryObserver<
|
#observer: InfiniteQueryObserver<
|
||||||
ProxyFontsResponse,
|
ProxyFontsResponse,
|
||||||
@@ -45,6 +52,8 @@ export class FontCatalogStore {
|
|||||||
constructor(params: FontStoreParams = {}) {
|
constructor(params: FontStoreParams = {}) {
|
||||||
this.#params = { limit: 50, ...params };
|
this.#params = { limit: 50, ...params };
|
||||||
this.#observer = new InfiniteQueryObserver(this.#qc, this.buildOptions());
|
this.#observer = new InfiniteQueryObserver(this.#qc, this.buildOptions());
|
||||||
|
// Seed result synchronously; subscribe may not fire on disabled observers.
|
||||||
|
this.#result = this.#observer.getCurrentResult();
|
||||||
this.#unsubscribe = this.#observer.subscribe(r => {
|
this.#unsubscribe = this.#observer.subscribe(r => {
|
||||||
this.#result = r;
|
this.#result = r;
|
||||||
});
|
});
|
||||||
@@ -88,10 +97,13 @@ export class FontCatalogStore {
|
|||||||
return this.#result.error ?? null;
|
return this.#result.error ?? null;
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
* True if no fonts were found for the current filter criteria
|
* True if no fonts were found for the current filter criteria.
|
||||||
|
* Always false until the observer has been enabled (via setParams) — otherwise
|
||||||
|
* the UI would briefly render "no results" on mount before bindings configures
|
||||||
|
* the query.
|
||||||
*/
|
*/
|
||||||
get isEmpty(): boolean {
|
get isEmpty(): boolean {
|
||||||
return !this.isLoading && !this.isFetching && this.fonts.length === 0;
|
return this.#enabled && !this.isLoading && !this.isFetching && this.fonts.length === 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -129,10 +141,12 @@ export class FontCatalogStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Merge new parameters into existing state and trigger a refetch
|
* Merge new parameters into existing state and trigger a refetch.
|
||||||
|
* The first call also enables the observer (see `#enabled`).
|
||||||
*/
|
*/
|
||||||
setParams(updates: Partial<FontStoreParams>) {
|
setParams(updates: Partial<FontStoreParams>) {
|
||||||
this.#params = { ...this.#params, ...updates };
|
this.#params = { ...this.#params, ...updates };
|
||||||
|
this.#enabled = true;
|
||||||
this.#observer.setOptions(this.buildOptions());
|
this.#observer.setOptions(this.buildOptions());
|
||||||
}
|
}
|
||||||
/**
|
/**
|
||||||
@@ -431,6 +445,7 @@ export class FontCatalogStore {
|
|||||||
const next = lastPage.offset + lastPage.limit;
|
const next = lastPage.offset + lastPage.limit;
|
||||||
return next < lastPage.total ? { offset: next } : undefined;
|
return next < lastPage.total ? { offset: next } : undefined;
|
||||||
},
|
},
|
||||||
|
enabled: this.#enabled,
|
||||||
staleTime: hasFilters ? 0 : DEFAULT_QUERY_STALE_TIME_MS,
|
staleTime: hasFilters ? 0 : DEFAULT_QUERY_STALE_TIME_MS,
|
||||||
gcTime: DEFAULT_QUERY_GC_TIME_MS,
|
gcTime: DEFAULT_QUERY_GC_TIME_MS,
|
||||||
};
|
};
|
||||||
@@ -441,6 +456,11 @@ export class FontCatalogStore {
|
|||||||
try {
|
try {
|
||||||
response = await fetchProxyFonts(params);
|
response = await fetchProxyFonts(params);
|
||||||
} catch (cause) {
|
} catch (cause) {
|
||||||
|
// Preserve non-retryable validation errors so the query client doesn't
|
||||||
|
// burn the retry budget on a deterministic schema mismatch.
|
||||||
|
if (cause instanceof FontResponseError) {
|
||||||
|
throw cause;
|
||||||
|
}
|
||||||
throw new FontNetworkError(cause);
|
throw new FontNetworkError(cause);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -40,6 +40,10 @@ interface Props extends
|
|||||||
* Skeleton snippet
|
* Skeleton snippet
|
||||||
*/
|
*/
|
||||||
skeleton?: Snippet;
|
skeleton?: Snippet;
|
||||||
|
/**
|
||||||
|
* Empty-state snippet rendered when the query settled with zero fonts
|
||||||
|
*/
|
||||||
|
empty?: Snippet;
|
||||||
}
|
}
|
||||||
|
|
||||||
let {
|
let {
|
||||||
@@ -47,6 +51,7 @@ let {
|
|||||||
onVisibleItemsChange,
|
onVisibleItemsChange,
|
||||||
weight,
|
weight,
|
||||||
skeleton,
|
skeleton,
|
||||||
|
empty,
|
||||||
...rest
|
...rest
|
||||||
}: Props = $props();
|
}: Props = $props();
|
||||||
|
|
||||||
@@ -59,6 +64,8 @@ let isCatchingUp = $state(false);
|
|||||||
|
|
||||||
const showInitialSkeleton = $derived(!!skeleton && isLoading && fontCatalogStore.fonts.length === 0);
|
const showInitialSkeleton = $derived(!!skeleton && isLoading && fontCatalogStore.fonts.length === 0);
|
||||||
const showCatchupSkeleton = $derived(!!skeleton && isCatchingUp);
|
const showCatchupSkeleton = $derived(!!skeleton && isCatchingUp);
|
||||||
|
// Settled query with no matches — empty state replaces the (otherwise blank) list.
|
||||||
|
const showEmpty = $derived(!!empty && !isLoading && !isCatchingUp && fontCatalogStore.fonts.length === 0);
|
||||||
|
|
||||||
function handleInternalVisibleChange(items: UnifiedFont[]) {
|
function handleInternalVisibleChange(items: UnifiedFont[]) {
|
||||||
visibleFonts = items;
|
visibleFonts = items;
|
||||||
@@ -163,6 +170,10 @@ function handleNearBottom(_lastVisibleIndex: number) {
|
|||||||
<div class="overflow-hidden h-full" transition:fade={{ duration: 300 }}>
|
<div class="overflow-hidden h-full" transition:fade={{ duration: 300 }}>
|
||||||
{@render skeleton()}
|
{@render skeleton()}
|
||||||
</div>
|
</div>
|
||||||
|
{:else if showEmpty && empty}
|
||||||
|
<div class="h-full" transition:fade={{ duration: 200 }}>
|
||||||
|
{@render empty()}
|
||||||
|
</div>
|
||||||
{:else}
|
{:else}
|
||||||
<!-- VirtualList persists during pagination - no destruction/recreation -->
|
<!-- VirtualList persists during pagination - no destruction/recreation -->
|
||||||
<VirtualList
|
<VirtualList
|
||||||
|
|||||||
@@ -9,6 +9,7 @@
|
|||||||
|
|
||||||
import { api } from '$shared/api/api';
|
import { api } from '$shared/api/api';
|
||||||
import { API_ENDPOINTS } from '$shared/api/endpoints';
|
import { API_ENDPOINTS } from '$shared/api/endpoints';
|
||||||
|
import { NonRetryableError } from '$shared/api/queryClient';
|
||||||
|
|
||||||
const PROXY_API_URL = API_ENDPOINTS.filters;
|
const PROXY_API_URL = API_ENDPOINTS.filters;
|
||||||
|
|
||||||
@@ -37,7 +38,8 @@ export interface FilterMetadata {
|
|||||||
type: 'enum' | 'string' | 'array';
|
type: 'enum' | 'string' | 'array';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Available filter options
|
* Available filter options.
|
||||||
|
* Always an array; empty when the group has no options.
|
||||||
*/
|
*/
|
||||||
options: FilterOption[];
|
options: FilterOption[];
|
||||||
}
|
}
|
||||||
@@ -68,11 +70,16 @@ export interface FilterOption {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Proxy filters API response
|
* Proxy filters API response.
|
||||||
|
*
|
||||||
|
* Contract: `filters` (and each nested `options`) is always an array — never
|
||||||
|
* `null` or omitted. Wire-level `null` here is a backend regression and
|
||||||
|
* surfaces as a non-retryable error on the client.
|
||||||
*/
|
*/
|
||||||
export interface ProxyFiltersResponse {
|
export interface ProxyFiltersResponse {
|
||||||
/**
|
/**
|
||||||
* Array of filter metadata
|
* Array of filter metadata.
|
||||||
|
* Always an array; empty when no filter groups are configured.
|
||||||
*/
|
*/
|
||||||
filters: FilterMetadata[];
|
filters: FilterMetadata[];
|
||||||
}
|
}
|
||||||
@@ -99,7 +106,7 @@ export async function fetchProxyFilters(): Promise<FilterMetadata[]> {
|
|||||||
const response = await api.get<FilterMetadata[]>(PROXY_API_URL);
|
const response = await api.get<FilterMetadata[]>(PROXY_API_URL);
|
||||||
|
|
||||||
if (!response.data || !Array.isArray(response.data)) {
|
if (!response.data || !Array.isArray(response.data)) {
|
||||||
throw new Error('Proxy API returned invalid response');
|
throw new NonRetryableError('Proxy API returned invalid filters response');
|
||||||
}
|
}
|
||||||
|
|
||||||
return response.data;
|
return response.data;
|
||||||
|
|||||||
@@ -42,20 +42,19 @@ $effect.root(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Mirror filter selections + debounced search query into fontCatalogStore params.
|
* Mirror filter selections + debounced search query + sort into fontCatalogStore params.
|
||||||
|
*
|
||||||
|
* Filters and sort are merged into one setParams call to avoid a startup race:
|
||||||
|
* two separate effects each issued setOptions with a different queryKey on the
|
||||||
|
* first flush, producing an orphaned `?limit=50&offset=0` fetch immediately
|
||||||
|
* followed by the real `?limit=50&sort=popularity&offset=0` fetch.
|
||||||
|
*
|
||||||
* untrack the write so fontCatalogStore's internal $state reads don't feed back
|
* untrack the write so fontCatalogStore's internal $state reads don't feed back
|
||||||
* into this effect's dependency graph.
|
* into this effect's dependency graph.
|
||||||
*/
|
*/
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
const params = mapAppliedFiltersToParams(appliedFilterStore);
|
const params = mapAppliedFiltersToParams(appliedFilterStore);
|
||||||
untrack(() => fontCatalogStore.setParams(params));
|
const sort = sortStore.apiValue;
|
||||||
});
|
untrack(() => fontCatalogStore.setParams({ ...params, sort }));
|
||||||
|
|
||||||
/**
|
|
||||||
* Mirror sort selection into fontCatalogStore.
|
|
||||||
*/
|
|
||||||
$effect(() => {
|
|
||||||
const apiSort = sortStore.apiValue;
|
|
||||||
untrack(() => fontCatalogStore.setSort(apiSort));
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,5 +1,15 @@
|
|||||||
import { QueryClient } from '@tanstack/query-core';
|
import { QueryClient } from '@tanstack/query-core';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Marker base class for errors that retrying will never fix — schema-validation
|
||||||
|
* failures, unauthorized responses, contract violations, etc.
|
||||||
|
*
|
||||||
|
* The queryClient retry handler short-circuits when it sees this; without it,
|
||||||
|
* a non-transient backend bug pins the UI through the full retry budget
|
||||||
|
* (default 3× exponential backoff ≈ 7s).
|
||||||
|
*/
|
||||||
|
export class NonRetryableError extends Error {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Data remains fresh for this long after fetch. Stores that override
|
* Data remains fresh for this long after fetch. Stores that override
|
||||||
* staleness (e.g. filtered queries) can use 0 to bypass.
|
* staleness (e.g. filtered queries) can use 0 to bypass.
|
||||||
@@ -51,7 +61,12 @@ export const queryClient = new QueryClient({
|
|||||||
* Refetch on mount if data is stale
|
* Refetch on mount if data is stale
|
||||||
*/
|
*/
|
||||||
refetchOnMount: true,
|
refetchOnMount: true,
|
||||||
retry: QUERY_RETRY_COUNT,
|
retry: (failureCount, error) => {
|
||||||
|
if (error instanceof NonRetryableError) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return failureCount < QUERY_RETRY_COUNT;
|
||||||
|
},
|
||||||
/**
|
/**
|
||||||
* Exponential backoff: 1s, 2s, 4s, 8s... capped at 30s
|
* Exponential backoff: 1s, 2s, 4s, 8s... capped at 30s
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -104,6 +104,12 @@ function isFontReady(font: UnifiedFont): boolean {
|
|||||||
gap={2}
|
gap={2}
|
||||||
class="bg-transparent min-h-0 h-full scroll-stable py-2 pl-6 pr-4"
|
class="bg-transparent min-h-0 h-full scroll-stable py-2 pl-6 pr-4"
|
||||||
>
|
>
|
||||||
|
{#snippet empty()}
|
||||||
|
<div class="px-6 py-12 flex items-center justify-center">
|
||||||
|
<Label variant="muted" size="sm">No typefaces found</Label>
|
||||||
|
</div>
|
||||||
|
{/snippet}
|
||||||
|
|
||||||
{#snippet skeleton()}
|
{#snippet skeleton()}
|
||||||
<div class="py-2.5 md:py-3 px-7">
|
<div class="py-2.5 md:py-3 px-7">
|
||||||
{#each { length: 50 } as _, index (index)}
|
{#each { length: 50 } as _, index (index)}
|
||||||
|
|||||||
Reference in New Issue
Block a user