-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add data shim so jQuery's data functions also respect domData. #43
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is some great work. We need to change that .js
, and I recommend not pulling it in ... at least at this point for people who are using legacy, this could just throw a wrench in?
can-jquery.js
Outdated
@@ -11,6 +11,7 @@ var makeArray = require("can-util/js/make-array/make-array"); | |||
var mutate = require("can-util/dom/mutate/mutate"); | |||
var setImmediate = require("can-util/js/set-immediate/set-immediate"); | |||
var canViewModel = require("can-view-model"); | |||
require("./data.js"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.js
should probably be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should not include this right now, and document this module so people can add it. That way people can experiment with it? This might be a good way to start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestions @justinbmeyer . I've implemented them. Please review and approve.
I had to fix how events were dispatched to make the tests pass. |
Posting this for discussion of implementation (though the implementation should work for most cases). This PR adds support for jQuery prototype
.data()
and.removeData()
, and for static$.data()
,$.hasData()
, and$.removeData()
. In each case, the domData cache from can-util will be examined or updated when the corresponding jQuery data function is called.The jQuery data store takes precedence for getting named properties. For grabbing the full data object, the results from jQuery data and domData are merged together.
$.data()
/.data()
called as a setter and$.removeData()
/.removeData()
operate on both caches.Addresses #42