search

ZiadJ / knockoutjs-reactor

Recursively tracks changes within a view model no matter how deeply nested the observables are or whether they are nested within dynamically created array elements.
Other
74 stars 22 forks source link

get underlying observable if knockout-es5 #22

Closed jrsearles closed 9 years ago

jrsearles commented 9 years ago

fixes issue #16

add support for knockout-es5

ZiadJ commented 9 years ago

Wouldn't it be better not to use a function for that since it's not being used elsewhere and only takes up two lines of code? It would also be good to comment it so we know it is a knockout-es5 specific functionality.

jrsearles commented 9 years ago

Sure - i can make that change. I thought this would be more readable, but either way is fine.

Thinking about this more though i was wondering if it might be better to instead expose a "getter" function as an option. That way a user can supply their own method to retrieve the observable and you wouldn't have to "know" about other plugins.

Let me know if you'd rather go that route and i can make that change. Otherwise i'll tweak this and resubmit.

Thanks!

ZiadJ commented 9 years ago

Totally agree! This could have other uses as well, like ignoring a certain node if a false value is returned. So that would be the way to go. I suggest you target the beta version though since I'm about to phase out the older one.

jrsearles commented 9 years ago

Alright - sounds good. I'll close this PR and submit another shortly.