canjs / can-connect

Model layer utilities for every JavaScript framework! Assemble real-time, high performance, restful data connections.
https://canjs.com/doc/can-connect.html
MIT License
29 stars 16 forks source link

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

Closed ivospinheiro closed 5 years ago

ivospinheiro commented 5 years ago

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 commented 5 years ago

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)
  5. As Todo.prototype.constructor is Todo, this will effectively dispatch a can.onInstanceBoundChange event on Todo with Todo.prototype as the instance.

  6. 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 commented 5 years ago

Fixed by: https://github.com/canjs/can-event-queue/issues/20