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.

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

feat: add breakFunc argument to use break in the “map” function #312

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

moowoonsunghoon
Copy link

@moowoonsunghoon moowoonsunghoon commented Nov 14, 2024

Tip

The owner of this PR can publish a preview release by commenting /publish in this PR. Afterwards, anyone can try it out by running pnpm add radashi@pr<PR_NUMBER>.

Summary

This PR adds the breakFunc argument to use break in the “map” function.

Related issue, if any:

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed
  • Release notes in next-minor.md or next-major.md have been added, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1 Difference
M src/async/map.ts 149 +25 (+20%)

Footnotes

  1. Function size includes the import dependencies of the function.

@moowoonsunghoon moowoonsunghoon changed the title map function added breakFunc arguments fear: “breakFunc” argument to use “break” in the “map” function. Nov 14, 2024
@moowoonsunghoon moowoonsunghoon changed the title fear: “breakFunc” argument to use “break” in the “map” function. feat: “breakFunc” argument to use “break” in the “map” function. Nov 14, 2024
@moowoonsunghoon moowoonsunghoon changed the title feat: “breakFunc” argument to use “break” in the “map” function. feat: add breakFunc argument to use break in the “map” function. Nov 14, 2024
@moowoonsunghoon moowoonsunghoon changed the title feat: add breakFunc argument to use break in the “map” function. feat: add breakFunc argument to use break in the “map” function Nov 14, 2024
@aleclarson
Copy link
Member

Hey @moowoonsunghoon, thanks for the PR! I appreciate you taking the time to contribute.

I think this could be a useful addition, but the use case may not be super common. That said, we can keep the PR open to gauge interest from the community. In the meantime, you can achieve a similar result using a for loop:

const prices: number[] = []
for (const symbol of stockSymbols) {
  const price = await calculateStockPrice(symbol)
  prices.push(price)
  if (sum(prices) > 100) {
    break
  }
}

Personally, I find the for..of version more readable, but I'm curious to hear your thoughts on the matter. Let me know what you think!

Thanks again for the contribution. I look forward to seeing more from you in the future!

@aleclarson aleclarson added new feature This PR adds a new function or extends an existing one awaiting more feedback Wait on this until more people comment. labels Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting more feedback Wait on this until more people comment. new feature This PR adds a new function or extends an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants