Skip to content

refactor: replace for-in loops with Object.entries and enable guard-for-in - #90

Merged
dinwwwh merged 1 commit into
mainfrom
claude/exciting-turing-yddy8o
Sep 15, 2026
Merged

dinwwwh merged 1 commit into
mainfrom
claude/exciting-turing-yddy8o

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 15, 2026

Copy link
Copy Markdown
Member

Summary

Refactored all for-in loops throughout the codebase to use Object.entries() for more explicit and safer object iteration. This change improves code clarity and enables stricter linting rules.

Key Changes

  • aws-lambda/headers.ts: Converted 4 for-in loops to Object.entries() when iterating over headers and multiValueHeaders
  • aws-lambda/url.ts: Converted 2 for-in loops to Object.entries() when iterating over query parameters
  • shared/object.ts: Simplified hasAnyDefinedValue() to use Object.values() instead of for-in with key lookup
  • core/utils.ts: Converted for-in loop in mergeStandardHeaders() to Object.entries(), eliminating redundant variable assignment
  • fastify/response.ts: Converted for-in loop to Object.entries() when setting response headers
  • node/response.ts: Converted for-in loop to Object.entries() when setting response headers
  • eslint.config.js: Enabled guard-for-in ESLint rule to prevent future for-in usage without explicit checks

Implementation Details

  • All conversions maintain identical functionality while improving code readability
  • Destructuring syntax for (const [key, value] of Object.entries(...)) makes variable sources explicit
  • Eliminates potential issues with inherited properties from prototype chain
  • The new ESLint rule ensures this pattern is enforced going forward

https://claude.ai/code/session_01JkQExPkUtiRfuLoFyXpqGD

@pkg-pr-new

pkg-pr-new Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@90

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@90

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@90

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@90

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@90

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@90

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@90

commit: 624382e

…ith `Object.entries` and enable `guard-for-in`

Turn on ESLint's `guard-for-in` rule so any future `for...in` loop must
filter out inherited properties. Rather than adding `Object.hasOwn`
guards to the existing loops, rewrite them over `Object.entries` (or
`Object.values` where only the value is read), which iterates own
enumerable properties only and drops the separate value lookup.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkQExPkUtiRfuLoFyXpqGD
@dinwwwh
dinwwwh force-pushed the claude/exciting-turing-yddy8o branch from 1c8326d to 624382e Compare September 15, 2026 02:00
@dinwwwh dinwwwh changed the title Replace for-in loops with Object.entries() for safer iteration refactor(core,node,fastify,aws-lambda,shared): replace for-in loops with Object.entries and enable guard-for-in Sep 15, 2026
@dinwwwh dinwwwh changed the title refactor(core,node,fastify,aws-lambda,shared): replace for-in loops with Object.entries and enable guard-for-in refactor: replace for-in loops with Object.entries and enable guard-for-in Sep 15, 2026
@codspeed

codspeed Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/exciting-turing-yddy8o (624382e) with main (57b5cb3)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues found.

Reviewed changes — full PR (#90), all 7 files and the single commit 1c8326d.

  • eslint.config.js — enables guard-for-in as error; pnpm lint is clean, and a repo-wide grep confirms no for...in loops remain, so the rule cannot trip CI on existing code.
  • packages/aws-lambda/src/headers.ts — toStandardHeaders (2), getEventHeader (2), and toLambdaHeaders (1) switch to Object.entries destructuring.
  • packages/aws-lambda/src/url.ts — toStandardUrl's two loops destructure [key, values], dropping the repeated index lookup.
  • packages/core/src/utils.ts — mergeStandardHeaders pulls bValue straight from the entries tuple; Object.hasOwn still gates inherited keys.
  • packages/fastify/src/response.ts / packages/node/src/response.ts — response header loops converted, undefined filtering unchanged.
  • packages/shared/src/object.ts — hasAnyDefinedValue uses Object.values, matching the JSDoc ("own property with a defined value").

The refactor is behaviorally neutral for the objects these paths actually see (plain literals, JSON.parse results, and null-prototype Object.create(null) maps). The one real semantic shift — inherited enumerable properties are no longer visited — is the PR's stated goal and, given the Object.hasOwn guard in mergeStandardHeaders and the own-property intent of hasAnyDefinedValue, is a strict improvement. Verified locally: pnpm lint passes and the 324 tests in aws-lambda/core/shared/node/fastify pass.

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues found.

Reviewed changes — new head 624382e since the prior review.

The diff for 624382e is byte-for-byte identical to 1c8326d (verified by comparing the two formatted diffs); the only change is the commit subject. No new code to review — the prior approval stands.

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@dinwwwh
dinwwwh merged commit 1d20536 into main Sep 15, 2026
10 checks passed
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.

2 participants