Skip to content

Add cross-frame compatibility to knockout.mapping #167

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

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

Conversation

traviskaufman
Copy link

I was using knockout.mapping on a project where the plugin needed to be loaded within a sandboxed environment (in this case, an iframe) and noticed that it was in no way behaving as expected. This seemed to be due to two things:

  1. exports.getType was looking at an object's constructor to determine what kind of object it was.
  2. There was some use of the instanceof operator for type inference.

Because of these, the library would not work unless loaded within the same window context as the script in which it was being used. This PR fixes that issue by using Object.prototype.toString to find the internal [[Class]] property of the object (as per the ECMA spec) and return the proper value from there.

Note that with the current implementation getType will return "object" when passed null. I'm not sure if this is the behavior that you're looking for since in most cases a client doesn't treat a "null" property the same way it treats other objects, but I left it since it's outside the scope of this PR. Happy to add it though if you guys would like! I tried it out and ran the QUnit suite and everything still passed.

p.s. AWESOME knockout plugin! Made my life a whole lot easier for this project!

@traviskaufman
Copy link
Author

What's the word on this? Is anyone maintaining this codebase currently? I see PR's open from 4 months ago

@sagacity
Copy link
Collaborator

Hi Travis, I currently don't have the time to maintain this project. There has been some discussion going on here for a potential replacement. You are also welcome to fork and extend, of course!

@brigand
Copy link
Collaborator

brigand commented Sep 16, 2013

I'm merging this into my fork. instanceof causes more problems than cross-frame stuff. I've read several articles about type checking primitives, and this seems to be the most reliable method, and works on all browsers.

Btw, whitespace fixes are nice, but should be done on a separate commit (I'm guessing your editor/IDE did it automatically).

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.

3 participants