Skip to content
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

Utilizing Proxy api when possible-feasable? #336

Closed
capaj opened this issue Jun 17, 2016 · 7 comments
Closed

Utilizing Proxy api when possible-feasable? #336

capaj opened this issue Jun 17, 2016 · 7 comments

Comments

@capaj
Copy link
Member

capaj commented Jun 17, 2016

As we've discussed numerous times in regards to Proxies- it would make sense to use those instead of getters/setters.

My question is on you @mweststrate - is it feasible from your perspective to plugin proxies into current MobX codebase? So that when a browser supports it, MobX would set it's traps using Proxies instead of getters/setters? I haven't looked into the codebase very deeply, but to me, it seems like it could be done if we created Proxy versions for observablearray.ts and observableobject.ts compatible with the current API.

What do you think @mweststrate ? Worth a try?

@mweststrate
Copy link
Member

I think it is doable, on the other hand I am a bit weary to do to this as long as it cannot be standardized upon, currently MobX is already taking care of differences between TypeScript and Babel transpiled code, and I think using proxies where available introduces a separate variable in the equation. I think it is nice to do in some major version, so that for example more fundamental problems can be fixed with it, like observable arrays not being real arrays :)

That being said, always feel free to take a shot at it, if the changes appear to be small, it might interesting to adopt it early.

@marvinhagemeister
Copy link

Just a heads up: The GoogleChrome team has build a polyfill for Proxies: https://github.com/GoogleChrome/proxy-polyfill. At the time of this writing it does only support set, get, construct and apply, though.

@capaj
Copy link
Member Author

capaj commented Nov 13, 2016

At the time of this writing it does only support set, get, construct and apply, though.

You make it sound like it will support "defineProperty" trap in the future. It won't, that is impossible unfortunately.
IMHO mobx should for now just stick to it's own implementatin of get/set traps.
Some future version like MobX 5.0.0 could switch to just utilizing proxies and require them even on startup-throw an error when running in an environment without them. that way apps authors can decide whether to utilize older ES5 mobx or newer ES6 MobX depending on how old browsers they need to support.

@marvinhagemeister
Copy link

@capaj That makes sense. Thanks for clearing that up 👍

@mweststrate
Copy link
Member

My current idea is to introduce object = dynamicObject(object) and implement that using ES6 proxies. This allows plain objects to be used as observable objects, without the current limitation of not supporting dynamic keys.

I think that is a nice case to start working with Proxies, learn about their caveats, performance etc. It has a clear use case and advantage over the current getter / setter implementation, yet there is a fallback for those needing to support browsers without proxies (probably the most of us): ObservableMaps

Example:

const todos = dynamicObject({
  "a23bf": { completed: false}
})

autorun(() => console.log(Object.keys(todos)))
// prints "a23bf"

todos["a8235"] = { completed: true}
// prints "a23bf", "a8235", something that currently can only be achieved through observable maps

@msand
Copy link

msand commented Nov 25, 2016

Just in case you haven't seen these...
https://github.com/capaj/proxevable
https://gist.github.com/ebidel/1b553d571f924da2da06
https://github.com/anywhichway/proxy-observe
Where the last one is most interesting: "Object.deepObserve goes beyond the EcmaScript spec and implements the ability to observe an object and all its sub-objects with a single call."

@mweststrate
Copy link
Member

@msand thanks for the links!

Closing this issue now in favor of #652 and see also the discussion #649; in mobx 3 cloning will be default behavior which is a next step in the direction of proxies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants