Define getters run twice. setVal is missing on first run. #2230

Closed
marshallswain opened this Issue Feb 2, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@marshallswain
Member

marshallswain commented Feb 2, 2016

With the latest version of Can, all of my define getters are getting run twice. On the first run, setVal is absent. On the second, it is present.

See this JSBin

can.Component.extend({
  tag: "home-page",
  template: can.view('home-template'),
  viewModel: {
    define: {
      names: {
        get(val, setVal){

          console.log('ARGUMENT COUNT:' + arguments.length);

          // This check is now necessary
          if (setVal) {
            console.log('There you are, setVal!');
            return val;
          }

          console.log('no setVal');
          return [];
        }
      }
    }
  }
});

@marshallswain marshallswain changed the title from setVal is missing. to Define getters run twice, setVal is missing on first run. Feb 2, 2016

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 2, 2016

Contributor

Is that bound to a select field?

Contributor

daffl commented Feb 2, 2016

Is that bound to a select field?

@marshallswain

This comment has been minimized.

Show comment
Hide comment
@marshallswain

marshallswain Feb 2, 2016

Member

Not in the JSBin: http://jsbin.com/ruvako/edit?html,js,console,output

It's simply

{{names}}
Member

marshallswain commented Feb 2, 2016

Not in the JSBin: http://jsbin.com/ruvako/edit?html,js,console,output

It's simply

{{names}}

@marshallswain marshallswain changed the title from Define getters run twice, setVal is missing on first run. to Define getters run twice. setVal is missing on first run. Feb 2, 2016

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Feb 2, 2016

Contributor

Here is a simpler example:
http://jsbin.com/qayicihipa/edit?html,js,console,output

It doesn't seem to happen when you bind directly to the map before the stache binding.

The same behavior occurs if you try to read the property before binding it.

Contributor

dylanrtt commented Feb 2, 2016

Here is a simpler example:
http://jsbin.com/qayicihipa/edit?html,js,console,output

It doesn't seem to happen when you bind directly to the map before the stache binding.

The same behavior occurs if you try to read the property before binding it.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 3, 2016

Contributor

This is due to the return readButDontObserveCompute(computedAttr.compute); added in #1915.

Contributor

justinbmeyer commented Feb 3, 2016

This is due to the return readButDontObserveCompute(computedAttr.compute); added in #1915.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 3, 2016

Contributor

Instead of the solution for #1915, we should probably observe on the compute and not on the property itself. Basically flip the behavior around. Perhaps __get should check if it's a computed property and not call can.__observe around here: https://github.com/canjs/canjs/blob/minor/map/map.js#L246

Contributor

justinbmeyer commented Feb 3, 2016

Instead of the solution for #1915, we should probably observe on the compute and not on the property itself. Basically flip the behavior around. Perhaps __get should check if it's a computed property and not call can.__observe around here: https://github.com/canjs/canjs/blob/minor/map/map.js#L246

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 3, 2016

Contributor

A more simple test would probably have something like:

c = can.compute(function(){
  return m.attr("p");
})

Where p is a async defined property on map m.

Contributor

justinbmeyer commented Feb 3, 2016

A more simple test would probably have something like:

c = can.compute(function(){
  return m.attr("p");
})

Where p is a async defined property on map m.

@justinbmeyer justinbmeyer self-assigned this Feb 3, 2016

@daffl daffl added this to the 2.3.14 milestone Feb 5, 2016

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 5, 2016

Contributor

Caused by #2224

Contributor

daffl commented Feb 5, 2016

Caused by #2224

justinbmeyer added a commit that referenced this issue Feb 5, 2016

@justinbmeyer justinbmeyer referenced this issue Feb 5, 2016

Merged

double get #2239

@daffl daffl closed this in #2239 Feb 5, 2016

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