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

Fix problem when observing 2 objects in the same application #6

Merged
1 commit merged into from
May 31, 2013
Merged

Fix problem when observing 2 objects in the same application #6

1 commit merged into from
May 31, 2013

Conversation

warpech
Copy link
Collaborator

@warpech warpech commented May 28, 2013

There was a collision in beforeDict that caused all observed objects to use the same lookup key.

Only browsers without Object.observe support are affected.

This is because only a string can be a object key. If another type is given, implicit conversion takes place - which may lead to unexpected results:

var key = "firstName";
beforeDict[key]; //becomes beforeDict["firstName"]

var key = 2;
beforeDict[key]; //becomes beforeDict["2"]

var key = {firstName: "Marcin"};
beforeDict[key]; //becomes beforeDict["[object Object]"]

I am quite surprised TypeScript does not do anything to help to avoid such pitfalls.

Included in this pull request is a fix and test case.

… collision in the lookup table (beforeDict[obj] resolves to beforeDict["[object Object]"]
@warpech warpech mentioned this pull request May 28, 2013
@warpech
Copy link
Collaborator Author

warpech commented May 28, 2013

This pull request fixes issue #5

ghost pushed a commit that referenced this pull request May 31, 2013
Fix problem when observing 2 objects in the same application
@ghost ghost merged commit 5551b31 into Starcounter-Jack:master May 31, 2013
simontrigowhite pushed a commit to simontrigowhite/jquery-handsontable that referenced this pull request Dec 28, 2013
This pull request was closed.
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.

1 participant