CSS: Warn and fill jQuery.cssProps #291
Conversation
src/css.js
Outdated
| return ret; | ||
| }; | ||
|
|
||
| migrateWarnProp( jQuery, "cssProp", {}, "jQuery.cssProp is deprecated and has no effect" ); |
There was a problem hiding this comment.
cssProps, not cssProp. :-) The same in a few other places.
|
I'm opening a ticket for this in jQuery, it can't be landed as-is. |
Krinkle
left a comment
Krinkle
left a comment
There was a problem hiding this comment.
This will trigger a warning when accessing property "cssProps" of object jQuery. It will trigger for var a = jQuery.cssProps;, jQuery.cssProps = b;.
Indirectly, it will also trigger for var c = jQuery.cssProps[ key ] and jQuery.cssProps[ key ] = d, by being supersets of A and B. But as it stands, this code would trigger 2 warnings for access of jQuery.cssProps given jQuery core from before jquery/jquery#4005, and would trigger 1 warning given jQuery core from after jquery/jquery#4005.
Is there a way we can do a catch-all for properties being set within this object? From the reply at jquery/jquery#4005, it sounds like you have something in mind, but aside from the old non-standard setters and too-new ES6 Proxy, I don't know of a way.
|
I'm not landing this pull request. was planning on using a proxy. I have a
proof-of-concept on JSbin but I can't get to it right now because I'm on my
phone.
… |
|
I still need to do a perf comparision. Hoping that getting will stay pretty fast and it doesn't really matter if setting gets slower since it shouldn't happen often at all. |
Krinkle
dismissed
their stale review
Looks good. I supppse it’ll be conditional on Proxy being available, without fallback?
Fixes jquery#283 Closes jquery#291 squash! Switch to Proxy for detecting writes
|
@Krinkle Yeah, I've updated the PR to use |
|
This shows that Proxy makes things "slower", even in the read case, but the numbers for both are so incredibly high that I'm not convinced they're right. If they are, the Proxy is certainly fast enough IMO. The most common case by far would be just reading the object so I'm not concerned about the read/write case at all, I just included it for completeness. |
Fixes jquery#283 Closes jquery#291 squash! Switch to Proxy for detecting writes
|
@dmethvin Why did you close this one? |
|
Whoops, I accidentally deleted the branch on my fork. I've got it locally and can push it back up though. |
Fixes jquery#283 Closes jquery#291 squash! Switch to Proxy for detecting writes
Fixes jquery#283 Closes jquery#291 squash! Switch to Proxy for detecting writes
|
Merged as 18618af |
3 participants
Fixes #283