Fix phone validation performance by using Set/Map instead of Array lookups#17843
Fix phone validation performance by using Set/Map instead of Array lookups#17843FelixMalfait merged 1 commit intomainfrom
Conversation
…okups `isValidCountryCode` was using `Array.includes()` (O(n)) on every call. `getCountryCodesForCallingCode` was iterating all ~250 countries and calling `getCountryCallingCode()` on each one per invocation. Both are now O(1) by precomputing a Set and a Map at module load time. Co-authored-by: Cursor <cursoragent@cursor.com>
Greptile OverviewGreptile SummaryThis PR optimizes phone validation performance by replacing expensive O(n) array operations with O(1) data structure lookups. Both Changes:
The optimization is sound and maintains behavioral equivalence. The precomputed map approach is appropriate since country calling codes are static data that doesn't change at runtime. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant transformPhonesValue
participant isValidCountryCode
participant getCountryCodesForCallingCode
participant Set/Map
Client->>transformPhonesValue: phone field mutation
transformPhonesValue->>isValidCountryCode: validate countryCode
isValidCountryCode->>Set/Map: Set.has(countryCode) - O(1)
Set/Map-->>isValidCountryCode: boolean
isValidCountryCode-->>transformPhonesValue: validation result
transformPhonesValue->>getCountryCodesForCallingCode: validate callingCode
getCountryCodesForCallingCode->>Set/Map: Map.get(callingCode) - O(1)
Set/Map-->>getCountryCodesForCallingCode: CountryCode[]
getCountryCodesForCallingCode-->>transformPhonesValue: country codes
transformPhonesValue-->>Client: validated phone data
|
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts">
<violation number="1" location="packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts:29">
P2: Returning the cached array exposes internal mutable state; callers can mutate it and corrupt future lookups. Return a copy to preserve previous behavior of returning a fresh array.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| return countryCallingCode === cleanCallingCode; | ||
| }); | ||
| return CALLING_CODE_TO_COUNTRIES.get(cleanCallingCode) ?? []; |
There was a problem hiding this comment.
P2: Returning the cached array exposes internal mutable state; callers can mutate it and corrupt future lookups. Return a copy to preserve previous behavior of returning a fresh array.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-shared/src/utils/validation/phones-value/getCountryCodesForCallingCode.ts, line 29:
<comment>Returning the cached array exposes internal mutable state; callers can mutate it and corrupt future lookups. Return a copy to preserve previous behavior of returning a fresh array.</comment>
<file context>
@@ -1,15 +1,30 @@
-
- return countryCallingCode === cleanCallingCode;
- });
+ return CALLING_CODE_TO_COUNTRIES.get(cleanCallingCode) ?? [];
};
</file context>
| return CALLING_CODE_TO_COUNTRIES.get(cleanCallingCode) ?? []; | |
| return [...(CALLING_CODE_TO_COUNTRIES.get(cleanCallingCode) ?? [])]; |
|
Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
1 similar comment
|
Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Summary
isValidCountryCodewas usingArray.includes()on ~250 country codes (O(n) per call). Replaced with aSet.has()lookup (O(1)).getCountryCodesForCallingCodewas iterating all ~250 countries and callinggetCountryCallingCode()on each one every invocation. Replaced with a precomputedMap<callingCode, CountryCode[]>built once at module load (O(1) per call).Both functions are called from
transformPhonesValueon every phone field mutation, causing cumulative overhead visible in profiling (p95 self-time ~20-35ms).Test plan
isValidCountryCodeorgetCountryCodesForCallingCodebehaviorMade with Cursor