Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: fix regressions caused by forEach to for..of migration #884

Merged
merged 2 commits into from
Jan 31, 2025
Merged

fix: fix regressions caused by forEach to for..of migration #884

merged 2 commits into from
Jan 31, 2025

Conversation

sidvishnoi
Copy link
Member

@sidvishnoi sidvishnoi commented Jan 31, 2025 •
edited
Loading

Context

Handles regressions from migrating from forEach to for..of loops in #814 (thankfully not in stores yet)
Extracted from #882

Changes proposed in this pull request

  • stray return when should've been a continue
  • refactor(MonetizationLinkManager): add onRemovedNode consistent with onAddedNode; replace some for..of with Array filter/map
  • addedNodes in frameManager should've been removedNodes (ref)
  • stronger, more consistent types

@github-actions github-actions bot added area: content Improvements or additions to extension content script area: background Improvements or additions to extension background script labels Jan 31, 2025
@sidvishnoi sidvishnoi changed the title fix: fix regressions caused by forEach to for..of` migration fix: fix regressions caused by forEach to for..of migration Jan 31, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 31, 2025 •
edited
Loading

Extension builds preview

Name Link
Latest commit 600c9b1
Latest job logs Run #13072012683
BadgeDownload
BadgeDownload

@sidvishnoi sidvishnoi mentioned this pull request Jan 31, 2025
Merged
@sidvishnoi sidvishnoi merged commit 4f1208d into main Jan 31, 2025
8 checks passed
@sidvishnoi sidvishnoi deleted the for-of-regressions branch January 31, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raducristianpopa raducristianpopa raducristianpopa approved these changes

@DarianM DarianM Awaiting requested review from DarianM

Assignees
No one assigned
Labels
area: background Improvements or additions to extension background script area: content Improvements or additions to extension content script
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sidvishnoi @raducristianpopa