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

.value functions should not be observable #162

Closed
justinbmeyer opened this issue Mar 18, 2017 · 6 comments
Closed

.value functions should not be observable #162

justinbmeyer opened this issue Mar 18, 2017 · 6 comments
Assignees
Labels

Comments

@justinbmeyer
Copy link
Contributor

If they are observable and invoked as part of a template, big problems can happen.

The solution is just to make sure

  1. Observation.add should not be called if this.__inSetup is true: https://github.com/canjs/can-construct/blob/master/can-construct.js#L280

  2. We should wrap value with Observation.ignore.

@justinbmeyer
Copy link
Contributor Author

http://jsbin.com/yafajal/edit?html,js,output

Also related ... we should check the number of dependencies in the observation.

@justinbmeyer
Copy link
Contributor Author

justinbmeyer commented Mar 18, 2017

Also, we can probably avoid .getCurrent() if this.__inSetup is true: https://github.com/canjs/can-define/blob/master/can-define.js#L280

here: canjs/can-reflect#2

@justinbmeyer
Copy link
Contributor Author

var DriversListVM = can.DefineMap.extend({
  drivers: {
    value: function(){
      return new can.DefineList([
        { title: "Dr.", first: "Cosmo", last: "Kramer", licensePlate: "A$$M^N" },
        { title: "Ms.", first: "Elaine", last: "Benes", licensePlate: "621433" }
     ])
    }
  },
  selected: {
    value: false
  },
  select: function(driver){
    this.selected = driver;
  }
})
var func = function(){};

func = Observation.ignore(func);

func();

@justinbmeyer
Copy link
Contributor Author

return function(newVal) {
  if( this._inSetup ) {
    setData.call(this, newVal);
  } else {

	var current = getCurrent.call(this);
	if (newVal !== current) {
	  setData.call(this, newVal);

	canEvent.dispatch.call(this, {
						type: prop,
						target: this
	  }, [newVal, current]);
	}
  }
}

@justinbmeyer
Copy link
Contributor Author

https://github.com/canjs/can-define/blob/master/can-define.js#L178

getInitialValue = Observation.ignore( make.get.defaultValue(prop, definition, typeConvert, eventsSetter) );

christopherjbaker pushed a commit that referenced this issue Mar 27, 2017
justinbmeyer added a commit that referenced this issue Mar 28, 2017
#162 .value functions should not be observable.
@bmomberger-bitovi
Copy link
Contributor

It turns out we couldn't avoid getCurrent because that was responsible for executing the getter applied by replaceWith before trying to set the value. However, that doesn't mean that we can't Observation.ignore it.

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

No branches or pull requests

3 participants