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: Alias merge as concat for Set and Map collections #1373

Merged
merged 2 commits into from
Oct 11, 2017

Conversation

leebyron
Copy link
Collaborator

These methods are extremely similar, but have differing implementations with different performance characteristics. This makes concat an alias of merge, sharing the implementation for those methods.

These methods are extremely similar, but have differing implementations with different performance characteristics. This makes `concat` an alias of `merge`, sharing the implementation for those methods.
@leebyron leebyron changed the title Alias merge as concat for Set and Map collections BREAKING: Alias merge as concat for Set and Map collections Oct 11, 2017
@leebyron leebyron merged commit c1656b1 into master Oct 11, 2017
@leebyron leebyron deleted the set-concat branch October 11, 2017 01:56
philipp-spiess added a commit to philipp-spiess/immutable-js that referenced this pull request Oct 18, 2017
immutable-js#1373 made `Map#concat` an alias for `Map#merge` but defined the alias
before `Map#merge` was defined, resulting in `Map#concat` being
undefined.

This fixes the issue and adds a regression test.
@philipp-spiess philipp-spiess mentioned this pull request Oct 18, 2017
leebyron pushed a commit that referenced this pull request Oct 18, 2017
* Fix Map#concat

#1373 made `Map#concat` an alias for `Map#merge` but defined the alias
before `Map#merge` was defined, resulting in `Map#concat` being
undefined.

This fixes the issue and adds a regression test.

* Alias on same line
@randallb
Copy link

I upgraded to RC9 and at runtime, orderedMaps can't concat. Concat returns as undefined. Working on a test case now.

@randallb
Copy link

Given a package.json:

{
  "name": "immmutable-bug-simple",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "immutable": "^4.0.0-rc.9"
  }
}

and an index.js of

const immutable = require('immutable');

const orderedMap = immutable.OrderedMap();
const orderedMap2 = immutable.OrderedMap();
orderedMap.concat(orderedMap2)

I get:

TypeError: orderedMap.concat is not a function
    at Object.<anonymous> (/Users/rb/code/experiment/immmutable-bug-simple/index.js:5:12)

@randallb
Copy link

Working to see if I can help write code to fix.

@randallb
Copy link

Opening a new issue too.

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