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

new can-connect map instance returns an instance initialized with previous object #441

Closed
ivospinheiro opened this issue Oct 8, 2018 · 2 comments

Comments

@ivospinheiro
Copy link

Please check the behavior of this sample code:
https://codepen.io/anon/pen/JmEdZR?editors=1011

The expected console output should be:

Object {
  id: 1,
  task: "Do something 1"
}
Object {
  id: 2,
  task: "Do something 2"
}
Object {
  id: 3,
  task: "Do something 3"
}

But the current result is:

Object {
  id: 1,
  task: "Do something 1"
}
Object {
  id: 3,
  task: "Do something 3"
}
Object {
  id: 3,
  task: "Do something 3"
}

Apparently it is somehow related with constructorStore behavior.

@justinbmeyer
Copy link
Contributor

So the problem with the following:

	var Todo = DefineMap.extend("Todo",{
		id: {identity: true, type: "number"},
		task: "string",
		get _serialize() {
			return this.serialize();
		}
	});
	const TodoList = DefineList.extend({
		"#": Todo
	});

	superMap({
		Map: Todo,
		List: TodoList,
		url: "/services/todos",
		name: "todo"
	});
	queues.log();
	var t1 = new Todo({ id: 1, task: "Do something 1" });
	t1.on("_serialize", () => {});
  1. We are listening to can.onInstanceBoundChange on the Todo type. This works by making all
    instances, when first bound, look to dispatch this sort of event on their constructor.

  2. When _serialize is bound, that will call getOwnEnumerableKeys which looks like:

    ObservationRecorder.add(this, 'can.keys');
    ObservationRecorder.add(Object.getPrototypeOf(this), 'can.keys');
  3. This will listen to can.keys on both the t1 instance and the Todo.prototype.

  4. Listening to can.keys on Todo.prototype will be the first "listener" on Todo.prototype. This will try to dispatch the can.dispatchInstanceBoundChange on the .constructor:

// can-event-queue
obj.constructor[dispatchBoundChangeSymbol](obj, true)
  1. As Todo.prototype.constructor is Todo, this will effectively dispatch a can.onInstanceBoundChange event on Todo with Todo.prototype as the instance.

  2. As it looks like an instance is bound, we will try to read it's .id property. To read the .id property, _data is read. As the instance is Todo.prototype, this will invoke the lazy getter for all future instances.

Solutions

The most obvious solution is to prevent binding changes on the prototype from dispatching can.onInstanceBoundChange events. I think we should check if obj.constructor.prototype === obj. If this is the case, we should not dispatch.

@justinbmeyer
Copy link
Contributor

Fixed by: canjs/can-event-queue#20

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

2 participants