Skip to content

fix(web): replace .map() with .forEach() in formalizePoliciesList - #7149

Open
vikash7485 wants to merge 3 commits into
pipe-cd:masterfrom
vikash7485:fix/formalize-policies-map-to-foreach
Open

fix(web): replace .map() with .forEach() in formalizePoliciesList#7149
vikash7485 wants to merge 3 commits into
pipe-cd:masterfrom
vikash7485:fix/formalize-policies-map-to-foreach

Conversation

@vikash7485

Copy link
Copy Markdown
Contributor

What this PR does:

Replace 4 misused .map() calls with .forEach() in formalizePoliciesList (web/src/utils/formalize-policies-list.ts).

Why we need it:

.map() was used at 4 locations purely for side effects (.push() into outer arrays). Each .map() call silently creates and discards an intermediate [undefined, undefined, ...] array, wasting memory. Using .forEach() is semantically correct and avoids the unnecessary allocation.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

No. This is a semantic code correctness fix with no behavioral change.

  • How are users affected by this change: No user-facing impact.
  • Is this breaking change: No.
  • How to migrate (if breaking change): N/A.

.map() was used at 4 locations in formalizePoliciesList purely for
side effects (.push() into outer arrays). Each .map() call silently
creates and discards an intermediate array of undefined values,
wasting memory. Using .forEach() is semantically correct.

Related: pipe-cd#6706
Signed-off-by: vikash7485 <vikkiraj073@gmail.com>
@vikash7485
vikash7485 force-pushed the fix/formalize-policies-map-to-foreach branch from c9a4c19 to 8d197e7 Compare August 11, 2026 04:01

@rahulshendre rahulshendre 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.

nice catch @vikash7485
btw what are your thoughts on using map().join() here 👀

@netlify

netlify Bot commented Aug 21, 2026

Copy link
Copy Markdown

Deploy Preview for pipecd-site canceled.

Name Link
🔨 Latest commit 95685e2
🔍 Latest deploy log https://app.netlify.com/projects/pipecd-site/deploys/6a88172433315400086ce68f

@vikash7485

vikash7485 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

@rahulshendre I think using map().join() is definitely cleaner and more idiomatic. It eliminates the mutable arrays (policies, resources, actions) and the manual trailing comma slicing (rsc.slice(0, -1)).

I'd be happy to refactor it to a fully declarative .map().join() pattern in this PR if you prefer that approach. Let me know and I can update the branch!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants