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

[BREAKING] Rename forEach to forSome, add forEach matching spec #1637

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

Conversation

conartist6
Copy link
Contributor

This is an attempt to make Immutable more spec compliant. Currently a naive user would assume that forEach has the same implementation as the forEach function on native types, which does not involve early exit.

Also since this forEach now returns this, it can be chained!

@conartist6 conartist6 changed the title Rename current forEach to forSome, add forEach Rename forEach to forSome, add forEach matching spec Oct 30, 2018
@leebyron leebyron changed the title Rename forEach to forSome, add forEach matching spec [BREAKING] Rename forEach to forSome, add forEach matching spec Oct 31, 2018
@conartist6
Copy link
Contributor Author

conartist6 commented Oct 31, 2018

@tolmasky This is one of those changes that's very relevant to our discussion of breaking changes. I am proposing it specifically after considering what I said to you about considering the es6 API of Map and Set to be a gold standard that can be relied upon to be stable across versions. I'm going to need to do some research on whether Immutable itself has ever relied on being able to leverage this particular behavior of forEach when using forEach as an instance method of an object whose Immutable version is unknown.

@bdurrer
Copy link
Contributor

bdurrer commented Oct 25, 2021

That function has bitten us more than once, this is a good idea.
But I fear it will bash us again because we might miss a few cases. So I wonder if we could/should either

  • have a migration-support-version of Immutable that logs warnings for every [potentially wrong] usage of a breaking feature
  • Split this in a two step operation: rename it to forSome in one version and only later reintroduce forEach

@conartist6
Copy link
Contributor Author

Two step sounds like the right thing to do to me.

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

Successfully merging this pull request may close these issues.

None yet

3 participants