Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Release accessing read function of observable #110

Closed
wants to merge 1 commit into from

2 participants

@developingjim

Tracked down a memory leak in my application that appears to originate
from the line. I was not able to reproduce this with core knockback and
it's dependencies. I was also not able to run the test suite on windows.
This is also my first commit to github. Given all that I won't be
offended by a rejection ;)

Thinking through it, there doesn't appear to be any reason why the
release function shouldn't be using peek here, but I can't prove that it
causes a problem outside of my application.

If you'd like more information about the approach we took with the app
I'd be happy to explain and share some code samples.

Also just wanted to say thanks, using knockback has helped greatly
reduce the amount of custom javascript code that I've seen backbone only
projects produce.

@developingjim developingjim Release accessing read function of observable
Tracked down a memory leak in my application that appears to originate
from the line. I was not able to reproduce this with core knockback and
it's dependencies. I was also not able to run the test suite on windows.
This is also my first commit to github. Given all that I won't be
offended by a rejection ;)

Thinking through it, there doesn't appear to be any reason why the
release function shouldn't be using peek here, but I can't prove that it
causes a problem outside of my application.

If you'd like more information about the approach we took with the app
I'd be happy to explain and share some code samples.

Also just wanted to say thanks, using knockback has helped greatly
reduce the amount of custom javascript code that I've seen backbone only
projects produce.
9e94161
@kmalakoff
Owner

I'll accept this pull request once knockout 3.1.0 is installable through npm (there is an open bug: knockout/knockout#1351). One change you should make for legacy support (earlier versions of knockout didn't have peek):

    if ko.isObservable(obj) and _.isArray(array = _peekObservable(obj))

I'm happy to hear that knockback reduces boilerplate code for you - that's one reason I wrote it!

@kmalakoff kmalakoff referenced this pull request from a commit
Kevin Malakoff #110 21c072e
@developingjim

Ahh, got it. Thanks for the response ;)

@kmalakoff
Owner

Thanks, I've checked it in

@kmalakoff kmalakoff closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 6, 2014
  1. @developingjim

    Release accessing read function of observable

    developingjim authored
    Tracked down a memory leak in my application that appears to originate
    from the line. I was not able to reproduce this with core knockback and
    it's dependencies. I was also not able to run the test suite on windows.
    This is also my first commit to github. Given all that I won't be
    offended by a rejection ;)
    
    Thinking through it, there doesn't appear to be any reason why the
    release function shouldn't be using peek here, but I can't prove that it
    causes a problem outside of my application.
    
    If you'd like more information about the approach we took with the app
    I'd be happy to explain and share some code samples.
    
    Also just wanted to say thanks, using knockback has helped greatly
    reduce the amount of custom javascript code that I've seen backbone only
    projects produce.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  src/knockback-core/knockback-core.coffee
View
2  src/knockback-core/knockback-core.coffee
@@ -99,7 +99,7 @@ class kb
# observable or lifecycle managed
if ko.isObservable(obj) or (typeof(obj.dispose) is 'function') or (typeof(obj.destroy) is 'function') or (typeof(obj.release) is 'function')
- if ko.isObservable(obj) and _.isArray(array = obj())
+ if ko.isObservable(obj) and _.isArray(array = obj.peek())
if obj.__kb_is_co or (obj.__kb_is_o and (obj.valueType() is KB_TYPE_COLLECTION))
if obj.destroy
obj.destroy()
Something went wrong with that request. Please try again.