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

Wrap nested objects with setters #110

Closed
gliese1337 opened this issue Aug 7, 2013 · 6 comments
Closed

Wrap nested objects with setters #110

gliese1337 opened this issue Aug 7, 2013 · 6 comments

Comments

@gliese1337
Copy link

Inspired by https://github.com/Rich-Harris/Ractive/issues/74
Just like arrays are wrapped so that array mutator methods are replaced by versions that properly update ractive data, it would be nice if all nested property objects had getter/setter methods defined, so that ractive.data.someProperty = 'foo' would act the same as ractive.set('someProperty','foo'), ractive.data.someProperty.subProperty = 'foo' would act the same as ractive.set('someProperty.subProperty','foo'), etc.

The .set and .get functions would still be around for those who need to ensure support for older browsers.

This would allow one to extract a subset of the data model to pass off to some other part of the application that doesn't need access to the entire ractive and still have changes properly reflected (which one currently can do with arrays, but nothing else).

@codler
Copy link
Member

codler commented Aug 8, 2013

I think that already works today.

@codler
Copy link
Member

codler commented Aug 8, 2013

nvm, it works for arrays and not "strings" http://jsfiddle.net/ygQd6/

@Rich-Harris
Copy link
Member

Yeah, there was a discussion about this on Hacker News - I'll copy and paste what I wrote there:


It's definitely something I've thought about. There are a few reasons I still prefer ractive.set( keypath, value ), which I'll try and articulate:

  1. It's explicit, and therefore more predictable. I get itchy when libraries take control out of my hands. Though I understand not everyone feels that way!
  2. Performance. Getters and setters have a penalty (or did the last time I looked into it in any depth).
  3. It only works for existing properties. With Ractive you don't have to declare the 'shape' of your entire model up-front - you can start with a completely empty model and set properties as and when it's convenient. (With ES6 and Object.observe maybe we can sidestep that issue one day.)
  4. Performance (again). If you set several properties simultaneously (e.g. ractive.set({ opacity: 0.5, left: 10 }), or whatever), then updates won't happen until all the new data has been taken account of.
  5. That irritating thing that happens when you try to do foo.bar.baz = 'bob' and you get an error because foo.bar is undefined.
  6. There are some cases where you want to do something like ractive.set( keypath + '.complete', true ) - you can only do that with string-based keypaths.

As for emn13's suggestion of Knockout-style observables, my own experience is that having to inherit from custom observable classes gets cumbersome quite quickly. But different strokes for different folks!


So at the moment I don't foresee this ever being the default setting. But maybe there could be a magic: true initialisation option, or something. I certainly see the appeal (I had the 'ooh, shiny!' moment when I first played with Angular, and the example of passing data off to a non-Ractive-aware part of the app is a good one).

@gliese1337
Copy link
Author

3 & 5 don't concern me much- I don't see much reason to ever set properties that are neither referenced in the template nor present in the initial data. For all the rest, I totally agree! There are lots of situations where I want to use .set() instead of an implicit setter, and if only for performance I would never consider getting rid of that option (and I'm not a big fan of Knockout observables, either).

But I like this as an additional option for interacting with the data. A magic: true initialization setting seems like a good idea.

@Rich-Harris
Copy link
Member

Okay, I've had a go at this. Fair warning: I've only tested it in Chrome, and I haven't done any tests to see what the performance impact is (if you do magic: true, then every property will be wrapped in a getter/setter pair, even if you only interact via ractive.set() and ractive.get().

But I've tried it out and it seems to do the trick. Let me know how you get on. As mentioned earlier in the thread, this will only work with properties that are already extant on the model (if you can think of a non-hacky workaround, I'm all ears).

Rich-Harris added a commit that referenced this issue Aug 19, 2013
…apped once, even if a property is referenced by multiple ractives on multiple keypaths
@Rich-Harris
Copy link
Member

Magic mode is now documented on the wiki - closing this issue

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

No branches or pull requests

3 participants