Skip to content

subCursor (cursor.cursor('a')) returns improper type #381

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

Merged
merged 2 commits into from
Mar 27, 2015

Conversation

jeffbski
Copy link
Contributor

@jeffbski jeffbski commented Mar 9, 2015

Due to defect in the code, calling cursor() on cursor can return improper type

var data = Immutable.fromJS({ a: [1, 2, 3] });
var cursor = Cursor.from(data);
var deepCursor = cursor.cursor('a');
deepCursor instanceof IndexedCursor; // fails since is KeyedCursor

Originally reported here #297 I decided that it might be a safer fix to continue using argument.length rather than checking undefined since I think it might be valid to use undefined in some circumstances.

So this fix goes back to using argument.length in the makeCursor check, but properly calls it with only 3 arguments from subCursor if subCursor is not called with value.

jeroencranendonk-wf and others added 2 commits March 9, 2015 16:15
Certain code paths call makeCursor with four arguments, with 'value'
undefined. Since arguments.length is still four in this case, the
value is not retrieved from rootData, and an incorrect cursor type can
be generated.
When calling cursor.cursor(), the subCursor fn calls makeCursor and its logic expects arguments.length less than 4 to trigger making subCursor.

Unfortunately subCursor calls it with 4 arguments, so we can correct this so that it will call with 3 or 4 depending on how it was called.

This is an alternate fix to @jcranendonk's since the other fix checked if value was void 0 (undefined), however it might be valid to pass undefined to these functions, so the argument.length check is probably safer.
@jeffbski
Copy link
Contributor Author

This issue is also mentioned in #318

@wmertens
Copy link
Contributor

👍 this fixes #398 as well.

@leebyron
Copy link
Collaborator

Thanks for your patience, this is a great fix.

leebyron added a commit that referenced this pull request Mar 27, 2015
subCursor (cursor.cursor('a')) returns improper type
@leebyron leebyron merged commit 577c9a0 into immutable-js:master Mar 27, 2015
@jeffbski
Copy link
Contributor Author

Great! Thanks.

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

Successfully merging this pull request may close these issues.

4 participants