perf(js): getValueFromKeysInArray optimization - #29798
Conversation
carlotestor
left a comment
There was a problem hiding this comment.
Summary
Rewrites getValueFromKeysInArray from Array.find + prop to a straight for…of loop in ts/src/base/functions/type.ts (JS/TS safe*N hot path only — Py/PHP keep their own helpers). Draft, empty body, +13/−1.
Correctness
I diffed old vs new on the usual edges (null/'' skip, 0/false keep, missing keys, nullish key entries, arrays as objects). Behavior matches prop's filter for the cases safe*N actually hit.
One intentional divergence (good): when no listed key hits, old code did object[array.find(…)] with find → undefined, which in JS is object['undefined'] and could return a real value if that key existed. The new loop returns undefined cleanly. Worth a one-liner in the PR body so reviewers don't treat it as accidental drift.
No new test.safeMethods cases; existing N-variant coverage still exercises this helper. A tiny case for the undefined-key footgun would lock the fix in if you want belt-and-suspenders before unmarkedraft.
Notes
- Drop the blank line inside the arrow body (neighbors
prop/prop2stay dense). k == nullworks; matchingprop'sk === undefined || k === nullwould keep the file consistent.- No numbers/bench in the description — a one-line microbench (or even “hot path under safeStringN”) would make the
perf(js)claim reviewable.
Labels: enhancement, javascript
Verdict: COMMENT (draft). Looks like a sound micro-opt + small correctness tidy; not blocking on style nits.
Automated triage by carlotestor (Hermes ccxt-new-pr-issue-webhook).
|
@kroitor @carlosmiei ready |
There was a problem hiding this comment.
Pull request overview
Optimizes getValueFromKeysInArray in the base type helpers to reduce overhead when selecting the first defined value from a list of candidate keys. This is a core utility used by safe* N helpers and therefore affects behavior/performance across the codebase (and transpiled targets).
Changes:
- Replaces the prior
Array.find(...)+prop(...)approach with a manual loop to reduce allocations/callback overhead. - Adds explicit early returns for non-object inputs and skips null/undefined keys during iteration.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Merge digest
flowchart LR
type["type"]
@@ -32,7 +32,19 @@
-const getValueFromKeysInArray = (object: Dictionary<any>, array: any[]) => isObject (object) ? object[array.find ((k: NullableIndexType) => prop (object, k) !== undefined)] : undefined;
+const getValueFromKeysInArray = <T>(
+ object: Dictionary<any>,
+ keys: any[],
+): T | undefined => {
+ if (!isObject (object)) return undefined;
+ for (const k of keys) {
+ if (k === undefined || k === null) continue;
+ const v = object[k];
+ if (v !== undefined && v !== null && (v as unknown) !== '') return v;
+ }
+ return undefined;
+}; |
Uh oh!
There was an error while loading. Please reload this page.