GoogleChrome / proxy-polyfill

Proxy object polyfill
Apache License 2.0
1.14k stars 175 forks source link

calling seal on the object passed to Proxy prevents extension. #48

Closed trusktr closed 5 years ago

trusktr commented 6 years ago

This is undesirable, because for example I might be making a Proxy on an object that will be used as a prototype and I still want users to be able to modify the prototype (mainly they can add their own props/methods).

Why does the object have to be sealed?

trusktr commented 6 years ago
     // The Proxy polyfill cannot handle adding new properties. Seal the target and proxy.

Does adding new properties break the Proxy? Or it just doesn't do anything with the new properties? What about deleting a property, does that make the Proxy have an error?

If the Proxy only silently ignores new properties, then there's no harm in not sealing, and I believe this is much more preferable. Sealing is highly anti-spec.

Instead, the Proxy should just have limitations, and those limitations should not affect that actual object. Sealing is such an impacting operation, it can completely break other code as soon as the Proxy is introduced, rather than only the Proxy-containing code failing. We should limit failure to the code that it trying to use the limited Proxy polyfill.

I think it's a much better idea not to seal the object passed to Proxy.

trusktr commented 6 years ago

Here's an idea. Instead of sealing the object, what if we give the user a non-standard API (because they are working with a non-spec-compliant polyfill anyways) that they can call to update the project.

For example:

const obj = {}
const p = new Proxy(obj, {...})

// Then, we want to add properties to the object, and for the Proxy to react to new properties:
obj.newProp = "foo"
Proxy.update(p) // update the proxy to handle new props.

This is much better than sealing. At least it gives some option on how to update the behavior of the Proxy without drastic possibility of breaking 3rd-party code. It's better to replace the non-standard seal behavior with this non-standard API, IMO.

samthor commented 6 years ago

If you're aware of all your changes it seems as if the Proxy might not be the right design pattern :)

But I guess there's some cases where this might be useful. If you wanted to contribute a change like you propose, I'd probably accept it. I'd call the method something like _refresh, and have the polyfill code add a stub where it found a native implementation.

samthor commented 5 years ago

Closing this because of my above comment. If you know about what changes you're making then the polyfill isn't really the right choice and I encourage you to write your own code.