refactor(typescript): tighten types and migrate core JS files to TS#7444
refactor(typescript): tighten types and migrate core JS files to TS#7444Shevilll wants to merge 1 commit into
Conversation
WalkthroughThe icon map now uses unquoted property names where valid, while preserving the same numeric IDs. A TypeScript ChangesIcon map normalization
Protected function helper
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/methods/helpers/protectedFunction.ts`:
- Around line 1-8: The protectedFunction helper only catches synchronous
exceptions, so promise-returning callbacks can still reject unhandled. Update
the wrapper in protectedFunction to detect when fn returns a promise from the
returned callback and attach a rejection handler (or await it in an async
wrapper) so both sync throws and async rejections are handled; use the existing
protectedFunction symbol and keep the current logging behavior in the catch
path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bed448f-eb96-4944-8c33-e208b53dbed3
📒 Files selected for processing (4)
app/containers/CustomIcon/mappedIcons.tsapp/lib/methods/helpers/protectedFunction.jsapp/lib/methods/helpers/protectedFunction.tsapp/reducers/index.ts
💤 Files with no reviewable changes (1)
- app/lib/methods/helpers/protectedFunction.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/helpers/protectedFunction.tsapp/containers/CustomIcon/mappedIcons.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/methods/helpers/protectedFunction.tsapp/containers/CustomIcon/mappedIcons.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/lib/methods/helpers/protectedFunction.tsapp/containers/CustomIcon/mappedIcons.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/helpers/protectedFunction.tsapp/containers/CustomIcon/mappedIcons.ts
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/CustomIcon/mappedIcons.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/helpers/protectedFunction.tsapp/containers/CustomIcon/mappedIcons.ts
🔇 Additional comments (1)
app/containers/CustomIcon/mappedIcons.ts (1)
2-233: LGTM!
| export default <T extends (...args: any[]) => any>(fn: T): ((...params: Parameters<T>) => void) => | ||
| (...params: Parameters<T>) => { | ||
| try { | ||
| fn(...params); | ||
| } catch (e) { | ||
| console.log(e); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle promise-returning callbacks too.
try/catch only covers synchronous throws. This helper is already used around an async stream handler in app/lib/services/connect.ts:153-166, so a rejection will bypass this catch and become an unhandled rejection. Based on learnings, this codebase expects floating promises to be handled explicitly.
♻️ Proposed fix
export default <T extends (...args: any[]) => any>(fn: T): ((...params: Parameters<T>) => void) =>
(...params: Parameters<T>) => {
try {
- fn(...params);
+ Promise.resolve(fn(...params)).catch((e) => {
+ console.log(e);
+ });
} catch (e) {
console.log(e);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default <T extends (...args: any[]) => any>(fn: T): ((...params: Parameters<T>) => void) => | |
| (...params: Parameters<T>) => { | |
| try { | |
| fn(...params); | |
| } catch (e) { | |
| console.log(e); | |
| } | |
| }; | |
| export default <T extends (...args: any[]) => any>(fn: T): ((...params: Parameters<T>) => void) => | |
| (...params: Parameters<T>) => { | |
| try { | |
| Promise.resolve(fn(...params)).catch((e) => { | |
| console.log(e); | |
| }); | |
| } catch (e) { | |
| console.log(e); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/methods/helpers/protectedFunction.ts` around lines 1 - 8, The
protectedFunction helper only catches synchronous exceptions, so
promise-returning callbacks can still reject unhandled. Update the wrapper in
protectedFunction to detect when fn returns a promise from the returned callback
and attach a rejection handler (or await it in an async wrapper) so both sync
throws and async rejections are handled; use the existing protectedFunction
symbol and keep the current logging behavior in the catch path.
Source: Learnings
1 participant
Summary of Changes
This PR tightens types and migrates several core untyped JavaScript files to strict TypeScript to enhance reliability, autocompletion, and compile-time correctness across the app.
protectedFunctionMigration (.js->.ts):app/lib/methods/helpers/protectedFunction.jsto TypeScript.<T extends (...args: any[]) => any>and theParameters<T>utility type, ensuring compile-time safety and autocompletion when wrapping callback functions.Root Redux Reducer Migration (
.js->.ts):app/reducers/index.jsto TypeScript.Mapped Icons Configuration Migration (
.js->.ts):app/containers/CustomIcon/mappedIcons.jsto TypeScript.as constmodifier on the mapped icons object to strongly type theTIconsNameunion type (keyof typeof mappedIcons). This ensures compile-time validation for icon names used throughout the entire codebase.Verification
pnpm lint(which runseslint . && tsc) and verified that the project compiles with absolutely zero type or formatting errors.Summary by CodeRabbit
Style
Bug Fixes