-
Notifications
You must be signed in to change notification settings - Fork 48
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
Date, RegExp, some Array prototype built-ins fail on proxies #6
Comments
Has there been any progress on this issue over the past ~year? |
Date.prototype.toString() got the auto-unwrapping in commit 84b1e99, immediately after this issue was filed. |
One reason why I haven't been pursuing this is that it's still unclear how proxies should interact with built-ins such as Date. It's not clear that the auto-unwrapping is the way to go (could also throw a TypeError). Patching all the Date.prototype methods to auto-unwrap should be easy enough to do, but I would first like to learn about your use case. It could help us understand what the right behavior should be. |
Our use case is that we're recording statistics on accesses to objects -- how many reads, writes, etc. I agree that Date and other built-ins are tricky. One possibility would be to map all of the "get_" and "to_" methods to the get trap (with the method name provide as the property name), and map all of the "set*" methods to the set trap. With this solution, in our use case the handler would record the desired statistics, then pass through a "set" using "targetname;", and pass through a "get" using "return targetname;". |
My last comment got mangled by the Markdown processor. It should say: then pass through a "set" using "target leftbracket name rightbracket(value);" and pass through a "get" using "return target leftbracket name rightbracket;", where I have used leftbracket and rightbracket to represent the corresponding characters. |
Presumably, if your use case is recording stats, you'll have the same issue with tracking access to things like DOM nodes, etc. It's a bit heavyweight for this shim to patch every browser-built-in object out there, so that probably won't happen. I would suggest that the best way to approach this seems to be to patch all the built-ins that you want to trace yourself, e.g. Date.prototype.setTime = (function(original) {
return function() {
// track stats: +1 for Date.prototype.setTime
return original(arguments);
}
}(Date.prototype.setTime)); An alternative could be to turn Date.prototype into a Proxy explicitly, like so: Date.prototype = new Proxy(Date.prototype, { ... }); then track access via its get and set traps. |
Hi Tom, Thanks for this suggestion. I think it would allow us to do what we want. As a follow-on: Is the limitation on Date proxies just in the shim On Tue, Apr 2, 2013 at 1:09 AM, Tom Van Cutsem notifications@github.comwrote:
|
Hi Bill, It's not surprising that you are confused: this part of the spec is still in flux and probably the least fleshed-out. We currently don't yet have a satisfactory solution for how a proxy should be able to acquire, or access, the "internal", private properties of objects like Date instances. Hence, the status quo is that you can't currently apply methods that expect true Date instances to proxies that wrap Date instances. The same holds for DOM types like NodeList. Originally (and as documented on the wiki page you linked to) the idea was simply to automatically "unwrap" proxies when passed to functions that expect e.g. a Date or a RegExp. While this would be fine for primitives like Date or RegExp, in general you don't want functions to auto-unwrap proxies, as proxies are also used as security wrappers: they may not want to provide direct access to the object they are wrapping. That's one reason why this auto-unwrapping of Dates and other primitives is not yet final. I'll update the wiki page to better reflect this history. |
Thanks for clearing up my confusion, Tom. I agree that auto-unwrapping seems undesirable, although I could imagine that it might be useful to allow a proxy to trigger unwrapping after it decides that an access is permitted. For the spec (as opposed to the shim), would you still consider it to be unreasonable to map the Date get* and to* methods to the "get" trap, and the Date set* methods to the "set" trap? This seems to me (perhaps naively) as the most natural way to handle this issue. You were concerned that doing this for Date/RegExp/etc. would imply that it should be done for the DOM types. But I'd argue that it's more important to do it for Date/RegExp/etc. Potentially any JavaScript code uses "Date" (even code running in non-browser environments such as node.js), whereas use of the DOM types is restricted to code that is manipulating the DOM. A restriction on the ability to apply proxies to Date objects seems like a significant limitation on the generality of proxies. Bill |
I agree with this, and this is a discussion that seems like it hasn't reached a conclusion yet on es-discuss. But it seems like that if |
Although I would note that the proposed |
In response to @billmark : I agree that Date and RegExp are different from the DOM and can deserve special treatment. W.r.t. mapping the Date get* and set* methods to the get/set trap, this is already what happens if you use the Date/the Proxy in a "normal" way. Consider: var d = new Date();
var p = new Proxy(d, { get : function(tgt, name, receiver){
// count +1
return tgt[name].bind(tgt);
} } );
p.getTime(); The last line of code will trigger the |
In response to @Benvie : Membranes would actually work fine with Dates or any other special built-in, because they properly forward to the W.r.t. the var d = new Date();
var p = new Proxy(d, { invoke : function(tgt, name, receiver, args){
// count +1
return tgt[name].apply(tgt, args);
} } );
p.getTime(); The benefit: no need to allocate a "bound function" anymore. |
After studying this issue some more, the [CallProperty] trap looks like the best solution for what we're doing. Here's a quick summary of my thinking in case it's useful as you design these features: The "get" example that Tom provided (thanks!) would work in the common cases, but doesn't do exactly what we want. For example, it has the same behavior for "Date.getTime()" as for "Date.setTime()", but we want to keep separate statistics for reads and writes. Fundamentally, the issue with the "get" approach is that it is trapping on the method lookup, not the actual method execution. With some more hackery, such as looking at the property name to see if it is "set*", we could get closer to the desired behavior in most cases but still wouldn't be recording exactly what we want. We could also return a wrapper function that increments our counter but that is getting quite messy. The [[CallProperty]]/invoke approach still requires us to special-case "Date", but that is acceptable for us. Thanks for all of your help Tom and Benvie; you're doing great work with Direct Proxies. |
Calling the builtin function 'sort' on an array wrapped in an ES6 proxy results in "illegal access" being thrown (a string, not an Error). This is probably related in some way to these issues: tvcutsem/harmony-reflect#6 tvcutsem/harmony-reflect#19 This change fixes the problem for the time being through a rather explicit workaround; if the issue also occurs for other functions, it will likely surface again. (also, the change in Session.js makes sure error log entry is nicely formatted even when strings are thrown)
In the direct proxies spec, built-ins on Date and RegExp should auto-unwrap the proxy:
Similarly for e.g. RegExp.prototype.exec and RegExp.prototype.test
Similarly also for the Date and RegExp toString() method, as explained elsewhere.
All non-generic Date.prototype and RegExp.prototype methods should be patched to auto-unwrap proxies.
The spidermonkey shell uses non-standard toSource() invocations that also currently fail on proxies. Those should probably also auto-unwrap.
The text was updated successfully, but these errors were encountered: