Reviewing Source Code Changes
Guide for reviewing PRs and source code changes in packages/ and frameworks/.
When to Use This Guide
- Reviewing pull requests modifying library source
- Validating implementation patterns before merging
- Checking code quality, types, documentation, and tests
Review Process
- Understand the change — Read PR description, identify affected files
- Check patterns — Verify code follows existing conventions
- Verify types — Ensure type safety and proper inference
- Review docs — Confirm JSDoc is complete and accurate
- Check tests — Validate runtime and type test coverage
What to Review
Code Quality
| Check |
Requirement |
| Naming |
Matches existing patterns (FormStore, useField, createForm) |
| Purity annotation |
// @__NO_SIDE_EFFECTS__ before pure factory functions |
| Import extensions |
All imports use .ts extension |
| Interface vs type |
Use interface for object shapes, type for unions/aliases |
| Folder structure |
Methods: name.ts, index.ts. Primitives/components in their folder |
Good — purity annotation:
// @__NO_SIDE_EFFECTS__
export function useField<TSchema, TFieldPath>(
form: FormStore<TSchema>,
config: UseFieldConfig<TSchema, TFieldPath>
): FieldStore<TSchema, TFieldPath> {
return {
/* ... */
};
}
Bad — missing annotation:
export function useField<TSchema, TFieldPath>(
form: FormStore<TSchema>,
config: UseFieldConfig<TSchema, TFieldPath>
): FieldStore<TSchema, TFieldPath> {
return {
/* ... */
};
}
Type Safety
| Check |
Requirement |
| Generic inference |
Types infer correctly without explicit annotations |
| Constraints |
Generic parameters have appropriate extends clauses |
| Return types |
Explicit return types on exported functions |
| Type tests |
.test-d.ts file covers type inference scenarios |
Good — constrained generic:
export function useField<
TSchema extends FormSchema,
TFieldPath extends RequiredPath<TSchema>,
>(
form: FormStore<TSchema>,
config: UseFieldConfig<TSchema, TFieldPath>
): FieldStore<TSchema, TFieldPath>;
Documentation
| Check |
Requirement |
| JSDoc present |
All exported functions have JSDoc |
| First line |
Action verb matching function purpose (see below) |
@param tags |
Every parameter documented |
@returns tag |
Return value documented |
| Overloads |
Every overload has its own complete JSDoc block |
First line patterns by category:
| Category |
Pattern |
| Primitives |
Creates a ... |
| Methods |
Focuses ..., Resets ..., Validates ... |
| Components |
Renders a ... |
| Utilities |
Returns ..., Gets ..., Sets ... |
Tests
| Check |
Requirement |
| Runtime tests |
.test.ts covers success cases, failure cases, edge cases |
| Type tests |
.test-d.ts validates type inference with expectTypeOf |
| Error handling |
Tests verify correct error messages and validation |
Common Issues
| Issue |
What to Look For |
| Missing purity annotation |
Factory function without // @__NO_SIDE_EFFECTS__ |
| Incomplete JSDoc |
Missing @param or @returns, wrong description format |
| No type tests |
New API without .test-d.ts file |
| Wrong import extension |
Imports without .ts suffix |
| Inconsistent naming |
Primitives not using create/use prefix, wrong Store suffix |
| Side effects in pure code |
Mutations, I/O, or global state in primitive/method creation |
Checklist
Related Skills
repo-structure-navigate — Navigate the codebaserepo-source-code-document — JSDoc requirements
