Skip to content

Commit a820ffd

Browse files
authored
Merge pull request #1946 from anyproto/feature/JS-8824-prevent-vault-reopen-on-resize
Feature/JS-8824: Prevent vault reopen on resize
2 parents b3fb446 + fec72de commit a820ffd

File tree

6 files changed

+254
-12
lines changed

6 files changed

+254
-12
lines changed

.claude/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
settings.local.json
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
---
2+
name: typescript-code-review
3+
description: Perform comprehensive code reviews for TypeScript projects, analyzing type safety, best practices, performance, security, and code quality with actionable feedback
4+
---
5+
6+
# TypeScript Code Review Skill
7+
8+
Perform thorough, professional code reviews for TypeScript code with focus on type safety, best practices, performance, security, and maintainability.
9+
10+
## Review Process
11+
12+
When reviewing TypeScript code, follow this structured approach:
13+
14+
### 1. Initial Assessment
15+
- Understand the code's purpose and context
16+
- Identify the scope (single file, module, feature, or entire codebase)
17+
- Note the TypeScript version and configuration (check `tsconfig.json`)
18+
- Review any relevant documentation or comments
19+
20+
### 2. Core Review Categories
21+
22+
#### Type Safety
23+
- **Strict mode compliance**: Verify `strict: true` in tsconfig.json and adherence
24+
- **Type annotations**: Check for proper type annotations, avoid implicit `any`
25+
- **Type narrowing**: Ensure proper use of type guards and narrowing
26+
- **Generic types**: Review generic usage for flexibility without sacrificing safety
27+
- **Union and intersection types**: Verify correct usage and handling
28+
- **Type assertions**: Flag unnecessary or dangerous type assertions (the `as` keyword and non-null assertion operator)
29+
- **Null/undefined handling**: Check for proper optional chaining (`?.`) and nullish coalescing (`??`)
30+
- **Return types**: Ensure all functions have explicit return types
31+
- **Discriminated unions**: Verify proper exhaustiveness checking
32+
33+
#### Code Quality & Best Practices
34+
- **Naming conventions**: Check for clear, descriptive names (camelCase for variables/functions, PascalCase for types/classes)
35+
- **Function length**: Flag functions longer than ~50 lines or with high complexity
36+
- **Single responsibility**: Ensure functions and classes have one clear purpose
37+
- **DRY principle**: Identify duplicate code that should be extracted
38+
- **Magic numbers/strings**: Flag hardcoded values that should be constants
39+
- **Error handling**: Review try-catch usage, error types, and error messages
40+
- **Async/await**: Check for proper async handling, avoid mixing callbacks/promises
41+
- **Immutability**: Prefer `const` over `let`, check for array/object mutations
42+
- **Enums vs unions**: Recommend const enums or union types over regular enums when appropriate
43+
44+
#### Modern TypeScript Features
45+
- **Optional chaining**: Suggest using `?.` for nested property access
46+
- **Nullish coalescing**: Recommend `??` over `||` for default values
47+
- **Template literal types**: Check for opportunities to use template literals
48+
- **Utility types**: Suggest `Partial`, `Pick`, `Omit`, `Record`, etc. where appropriate
49+
- **Const assertions**: Recommend `as const` for literal types
50+
- **Type predicates**: Use for custom type guards
51+
- **`satisfies` operator**: Use instead of type assertions when validating types
52+
53+
#### Performance
54+
- **Unnecessary re-renders**: In React/frameworks, check for memo usage, dependency arrays
55+
- **Large bundle imports**: Flag entire library imports when tree-shaking is possible
56+
- **Inefficient algorithms**: Identify O(n²) or worse when better options exist
57+
- **Memory leaks**: Check for cleanup in event listeners, subscriptions, timers
58+
- **Lazy loading**: Suggest dynamic imports for large modules
59+
- **Type calculation cost**: Flag extremely complex type calculations that slow compilation
60+
61+
#### Security
62+
- **Input validation**: Ensure user input is validated and sanitized
63+
- **XSS vulnerabilities**: Check for unsafe HTML rendering or `eval` usage
64+
- **Sensitive data**: Flag hardcoded secrets, tokens, or passwords
65+
- **Dependency vulnerabilities**: Recommend running `npm audit` or checking dependencies
66+
- **Type safety as security**: Ensure types prevent security issues (e.g., SQL injection through tagged templates)
67+
68+
#### Testing & Maintainability
69+
- **Test coverage**: Note missing tests for critical paths
70+
- **Type-only imports**: Use `import type` for type-only imports
71+
- **Circular dependencies**: Flag circular imports
72+
- **Barrel exports**: Check for performance issues with index files
73+
- **Documentation**: Verify JSDoc comments for public APIs
74+
- **Deprecation notices**: Ensure deprecated code is properly marked
75+
76+
### 3. Output Structure
77+
78+
Organize the review with clear sections:
79+
80+
```markdown
81+
## Summary
82+
[High-level overview: overall code quality, main concerns, highlights]
83+
84+
## Critical Issues 🔴
85+
[Issues that must be fixed: type errors, security vulnerabilities, breaking bugs]
86+
87+
## Important Improvements 🟡
88+
[Significant issues affecting maintainability, performance, or best practices]
89+
90+
## Suggestions 🔵
91+
[Nice-to-have improvements, style preferences, optimizations]
92+
93+
## Positive Observations ✅
94+
[What the code does well, good patterns to reinforce]
95+
96+
## Detailed Findings
97+
98+
### [Category 1: e.g., Type Safety]
99+
**File**: `path/to/file.ts:line_number`
100+
- **Issue**: [Description]
101+
- **Current code**:
102+
```typescript
103+
[code snippet]
104+
```
105+
- **Recommended**:
106+
```typescript
107+
[improved code]
108+
```
109+
- **Reasoning**: [Why this matters]
110+
111+
[Repeat for each finding]
112+
```
113+
114+
### 4. Code Review Guidelines
115+
116+
**Tone and Style**:
117+
- Be constructive and specific, not vague or critical
118+
- Explain the "why" behind recommendations
119+
- Provide code examples for suggested changes
120+
- Acknowledge good practices when present
121+
- Use severity indicators (🔴 critical, 🟡 important, 🔵 suggestion)
122+
123+
**Prioritization**:
124+
1. Critical: Security issues, type errors, runtime bugs
125+
2. Important: Performance problems, maintainability issues, anti-patterns
126+
3. Suggestions: Style improvements, modern syntax, optimizations
127+
128+
**Context Awareness**:
129+
- Consider the project's maturity (prototype vs production)
130+
- Respect existing patterns if consistent across codebase
131+
- Note tradeoffs (e.g., performance vs readability)
132+
- Reference the project's TypeScript configuration
133+
134+
### 5. Reference Files
135+
136+
For detailed guidance on specific topics, consult the reference files:
137+
138+
- `references/type-safety-checklist.md` - Comprehensive type safety review points
139+
- `references/common-antipatterns.md` - TypeScript anti-patterns to avoid
140+
- `references/security-checklist.md` - Security considerations for TypeScript
141+
- `references/performance-tips.md` - Performance optimization strategies
142+
143+
Search references using Grep when encountering specific issues. For example:
144+
- Type guard issues: grep "type guard" in `references/type-safety-checklist.md`
145+
- Performance concerns: grep "performance" in `references/performance-tips.md`
146+
147+
### 6. TypeScript Configuration Review
148+
149+
When reviewing `tsconfig.json`, check for:
150+
151+
**Recommended strict settings**:
152+
```json
153+
{
154+
"compilerOptions": {
155+
"strict": true,
156+
"noUncheckedIndexedAccess": true,
157+
"noImplicitOverride": true,
158+
"noPropertyAccessFromIndexSignature": true,
159+
"exactOptionalPropertyTypes": true,
160+
"noFallthroughCasesInSwitch": true,
161+
"noImplicitReturns": true,
162+
"noUnusedLocals": true,
163+
"noUnusedParameters": true
164+
}
165+
}
166+
```
167+
168+
### 7. Framework-Specific Considerations
169+
170+
**React + TypeScript**:
171+
- Component prop types with interfaces
172+
- Proper typing for hooks (`useState`, `useEffect`, `useCallback`, etc.)
173+
- Event handler types (e.g., `React.MouseEvent<HTMLButtonElement>`)
174+
- Ref types (`useRef<HTMLDivElement>(null)`)
175+
- Children typing (`React.ReactNode` vs `React.ReactElement`)
176+
177+
**Node.js + TypeScript**:
178+
- Proper types for Express/Fastify handlers
179+
- Async error handling in middleware
180+
- Environment variable typing
181+
- Database query result typing
182+
183+
**Testing**:
184+
- Type-safe mocks and stubs
185+
- Proper typing for test utilities (Jest, Vitest, etc.)
186+
- Type assertions in tests
187+
188+
### 8. Automated Checks to Recommend
189+
190+
Suggest running these tools if not already in use:
191+
- **TypeScript compiler**: `tsc --noEmit` for type checking
192+
- **ESLint**: With `@typescript-eslint/parser` and recommended rules
193+
- **Prettier**: For consistent formatting
194+
- **ts-prune**: Find unused exports
195+
- **depcheck**: Find unused dependencies
196+
- **madge**: Detect circular dependencies
197+
198+
### 9. Review Workflow
199+
200+
1. **Scan for critical issues first**: Type errors, security issues, obvious bugs
201+
2. **Review architecture**: File structure, module boundaries, separation of concerns
202+
3. **Deep dive into logic**: Algorithm correctness, edge cases, error handling
203+
4. **Check types thoroughly**: Accuracy, safety, appropriate use of TypeScript features
204+
5. **Performance review**: Identify bottlenecks, unnecessary work, optimization opportunities
205+
6. **Style and consistency**: Naming, formatting, pattern adherence
206+
7. **Testing and docs**: Coverage, clarity, maintainability
207+
208+
### 10. Example Interaction
209+
210+
**User**: "Review this TypeScript file for issues"
211+
212+
**Response Flow**:
213+
1. Read the file(s) provided
214+
2. Check for any `tsconfig.json` in the project
215+
3. Perform systematic review across all categories
216+
4. Structure findings with severity levels
217+
5. Provide specific, actionable recommendations with code examples
218+
6. Highlight positive practices
219+
7. Suggest next steps (run specific tools, add tests, refactor specific areas)
220+
221+
## Best Practices
222+
223+
- **Be thorough but practical**: Focus on issues that matter
224+
- **Provide context**: Explain why something is an issue and what problems it could cause
225+
- **Show, don't just tell**: Include code examples for recommendations
226+
- **Consider the audience**: Adjust detail level based on the team's TypeScript experience
227+
- **Stay current**: Reference modern TypeScript features (4.9+, 5.0+)
228+
- **Balance**: Don't let perfect be the enemy of good—acknowledge tradeoffs
229+
230+
## When to Use This Skill
231+
232+
Activate this skill when the user:
233+
- Explicitly asks for a code review of TypeScript code
234+
- Requests feedback on TypeScript implementation
235+
- Asks to check code for issues, bugs, or improvements
236+
- Wants to ensure TypeScript best practices are followed
237+
- Needs help improving code quality or type safety
238+
- Requests a security or performance audit of TypeScript code

.gitignore

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,4 @@ RELEASE_NOTES.md
3232
/darwin-arm*/
3333
/windows/
3434
.aider*
35-
.env
36-
37-
.claude/
35+
.env

src/ts/component/sidebar/left.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,11 @@ const SidebarLeft = observer(forwardRef<SidebarLeftRefProps, {}>((props, ref) =>
180180
$(window).off('mousemove.sidebar mouseup.sidebar');
181181

182182
const w = Math.max(0, (e.pageX - ox.current));
183+
const data = sidebar.getData(panel);
183184

184-
sidebar.setWidth(panel, false, w, true);
185+
if (!data.isClosed) {
186+
sidebar.setWidth(panel, false, w, true);
187+
};
185188
window.setTimeout(() => movedX.current = false, 15);
186189
};
187190

src/ts/lib/keyboard.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ class Keyboard {
9595
if (!data.isClosed && (cw <= threshold)) {
9696
sidebar.leftPanelClose(false, false);
9797
} else
98-
if (data.isClosed && (cw > threshold + data.width)) {
98+
if (data.isClosed && !data.savedClosed && (cw > threshold + data.width)) {
9999
sidebar.leftPanelOpen(data.width, false, false);
100100
};
101101
};

src/ts/lib/sidebar.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { analytics, I, J, keyboard, S, Storage, U } from 'Lib';
55
interface SidebarData {
66
width: number;
77
isClosed: boolean;
8+
savedClosed: boolean;
89
};
910

1011
/**
@@ -28,9 +29,9 @@ interface SidebarData {
2829
class Sidebar {
2930

3031
panelData = {
31-
[I.SidebarPanel.Left]: { width: 0, isClosed: false } as SidebarData,
32-
[I.SidebarPanel.SubLeft]: { width: 0, isClosed: false } as SidebarData,
33-
[I.SidebarPanel.Right]: { width: 0, isClosed: false } as SidebarData,
32+
[I.SidebarPanel.Left]: { width: 0, isClosed: false, savedClosed: false } as SidebarData,
33+
[I.SidebarPanel.SubLeft]: { width: 0, isClosed: false, savedClosed: false } as SidebarData,
34+
[I.SidebarPanel.Right]: { width: 0, isClosed: false, savedClosed: false } as SidebarData,
3435
};
3536

3637
isAnimating = false;
@@ -52,8 +53,9 @@ class Sidebar {
5253
const param = this.getSizeParam(panel);
5354
const width = this.limitWidth(panel, data.width || param.default);
5455
const isClosed = (undefined !== data.isClosed) && (panel != I.SidebarPanel.Right) ? Boolean(data.isClosed) : true;
56+
const savedClosed = Boolean(data.savedClosed);
5557

56-
this.setData(panel, isPopup, { width, isClosed }, false);
58+
this.setData(panel, isPopup, { width, isClosed, savedClosed }, false);
5759
this.setStyle(panel, isPopup, { width, isClosed });
5860

5961
if ((panel == I.SidebarPanel.Left) && !isPopup) {
@@ -69,7 +71,7 @@ class Sidebar {
6971
const key = [ panel, ns ].join('');
7072
const param = this.getSizeParam(panel);
7173

72-
return this.panelData[key] || { width: param.default, isClosed: true };
74+
return this.panelData[key] || { width: param.default, isClosed: true, savedClosed: false };
7375
};
7476

7577
/**
@@ -156,7 +158,7 @@ class Sidebar {
156158
pageWrapperLeft.addClass('sidebarAnimation');
157159
};
158160

159-
this.setData(I.SidebarPanel.Left, false, { isClosed: true }, save);
161+
this.setData(I.SidebarPanel.Left, false, { isClosed: true, ...(save ? { savedClosed: true } : {}) }, save);
160162
this.setStyle(I.SidebarPanel.Left, false, { width: 0, isClosed: true });
161163
this.resizePage(false, dataSubLeft.isClosed ? 0 : dataSubLeft.width, null, animate);
162164

@@ -187,7 +189,7 @@ class Sidebar {
187189
const dataSubLeft = this.getData(I.SidebarPanel.SubLeft);
188190

189191
this.setStyle(I.SidebarPanel.Left, false, { width, isClosed: false });
190-
this.setData(I.SidebarPanel.Left, false, { isClosed: false }, save);
192+
this.setData(I.SidebarPanel.Left, false, { isClosed: false, ...(save ? { savedClosed: false } : {}) }, save);
191193
this.resizePage(false, width + (dataSubLeft.isClosed ? 0 : dataSubLeft.width), null, animate);
192194

193195
analytics.event('ExpandVault');

0 commit comments

Comments
 (0)