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

[RFC] Remove some(), map(), reduce() functions #219

Merged
merged 3 commits into from
Jun 1, 2022

Conversation

clue
Copy link
Member

@clue clue commented May 17, 2022

This simple PR suggests removing the some(), map(), reduce() functions to reduce the API surface. With these changes applied, our API is pretty much identical to ES6 promises commonly used in JavaScript. This makes our APIs appear less complex and thus easier to understand.

Empirical evidence suggests these functions aren't used very commonly in our ecosystem either, so I'd rather use the chance to clean up our API surface with the upcoming Promise v3 release. I don't currently see a prominent need for these functions, but I'm open to the idea of reintroducing them later if needed. There's reason to believe that these functions could (should?) exist as part of our new reactphp/async package, especially considering how it mimics part of async.js.

I'm filing this as an RFC to get more feedback on these functions and to see how others feel about this. If this gets merged, I'll file a follow-up PR to deprecate these functions for Promise v2 to ease upgrading.

Refs #35

@clue
Copy link
Member Author

clue commented Jun 1, 2022

Rebased to resolve merge conflict now that #220 is in :shipit:

Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

👍

In case questions do come up, we can consider putting them in a @friends-of-reactphp package.

@WyriHaximus WyriHaximus merged commit 16052d3 into reactphp:3.x Jun 1, 2022
@clue clue deleted the less-functions branch June 2, 2022 07:47
@clue clue mentioned this pull request Jun 2, 2022
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.

3 participants