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

Immutable.Record instances should not be boxed #96

Open
Gozala opened this issue Apr 19, 2015 · 1 comment
Open

Immutable.Record instances should not be boxed #96

Gozala opened this issue Apr 19, 2015 · 1 comment

Comments

@Gozala
Copy link
Contributor

Gozala commented Apr 19, 2015

I have being running into a dropped missing unique "key" prop issue with Immutable.Record instances as they are boxed, I have posted a comment in the relevant change:
6d8b850#commitcomment-10791363

I there a reason why Immutable instances are also boxed and not just cursors ?

Also regardless of type of the structure that omniscient boxes it should probably use a key from that structure otherwise it is impossible to render data structure of that type with a key.

@mikaelbr
Copy link
Member

Immutable instances are boxed to unwrap them as arguments when passed to the render-function but still work as expected inside the shouldComponentUpdate.

You may have a point about using the key from the structure which is passed. For now it only checks the argument key or props.key. If we were to extend this, we would have to do some "precedence checking". What if argument key is set, and we have this:

if (_isCursor(props) || _isImmutable(props)) {
  inputCursor = props;
  _props = {};
  _props[_hiddenCursorField] = inputCursor;
}

if (inputCursor && inputCursor.get('key')) {
  key = inputCursor.get('key');
}

This would override key, passed in as argument, but that should probably have higher precedence. So we would have to do

if (!key && inputCursor && inputCursor.get('key')) {
  key = inputCursor.get('key');
}

But in this way we are starting to more and more couple to the Immutable.js implementation. _isCursor and _isImmutable are overridables, but inputCursor.get('key') would be implementation specific.

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

No branches or pull requests

2 participants