Skip to content

optimized pick() to eliminate intermediate array allocation#636

Open
khresth wants to merge 2 commits intomakenotion:mainfrom
khresth:main
Open

optimized pick() to eliminate intermediate array allocation#636
khresth wants to merge 2 commits intomakenotion:mainfrom
khresth:main

Conversation

@khresth
Copy link
Copy Markdown

@khresth khresth commented Oct 24, 2025

Description

Optimized the pick() function to improve performance by eliminating intermediate array allocations. The original implementation used keys.map() to create an array of [key, value] tuples, then converted it to an object via Object.fromEntries(). This created temporary data structures on every invocation.

The new implementation directly constructs the result object in a single loop, reducing memory allocations and iteration overhead. Since pick() is called twice per API request (for query params and body params), this optimization should improve performance across SDK operations.

How was this change tested?

  • Automated test (unit, integration, etc.)
  • Manual test (provide reproducible testing steps below)
bash
node node_modules\jest\bin\jest.js ./test

Test Results:

PASS test/id-extraction.test.ts
PASS test/helpers.test.ts
PASS test/Client.test.ts

Test Suites: 3 passed, 3 total
Tests: 24 passed, 24 total

Screenshots

image

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 24, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

@ksinder ksinder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty!!

@ksinder ksinder enabled auto-merge (squash) February 27, 2026 00:04
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the internal pick() helper in src/utils.ts to reduce per-call allocations/iteration overhead by replacing the keys.map() + Object.fromEntries() approach with a single-loop object construction. This supports overall SDK request performance since pick() is used when building request query/body params.

Changes:

  • Reimplemented pick() to build the result object via a for...of loop instead of creating intermediate [key, value] tuples.
  • Introduced a direct assignment-based implementation returning the accumulated object.

Comment on lines +18 to +21
const result = {} as Pick<O, K>
for (const key of keys) {
result[key] = (base as any)?.[key]
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pick() now uses (base as any)?.[key], introducing an any escape hatch that removes type safety and can mask real type errors. Consider restructuring the types so the loop can assign without any (e.g., build into a Partial<Pick<...>>/record shape and cast once at the end, or adjust the base parameter type to explicitly allow nullish values and keep the indexed access typed).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ Looks like a build error unfortunately, Error: 20:28 error Unexpected any. Specify a different type @typescript-eslint/no-explicit-any

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants