Items that need to be tackled for 2.0 release

bmomberger-bitovi commented 6 years ago

Last WIP for can-connect 2.0 (part of CanJS 4.0) is on https://github.com/canjs/can-connect/tree/major . The primary difficulties in getting any behaviors to work correctly with 4.0-release libraries are below:

can-model behavior dependencies
- The can/model behavior and several tests relied on behavior and implementation details of can-map and can-list. Trying to convert source and tests to use DefineMap and DefineList has been frustrating.
- Dispatching events on model classes doesn't seem to be working correctly.
- Shadowed ID doesn't work with DefineMap. Not really understanding what it is anyway
smart merge is hamstrung by reliance on can-map
- If continuing to support can-map (and thereby smart merge), event dispatching for can-map must be upgraded for queues. If instead changing smart merge to use DefineMap, other refactors will be necessary (probably around assign/update calls).
- If continuing to support can-map, can-map has the .compute function that returns a can-compute. One goal of this upgrade is removing all reference to can-compute, so this has to be rewritten or removed.
Expando symbol not working with can-map?
- can-reflect.set doesn't seem to work with symbol keys. I thought it did, but the map is rejecting it.
- As a result of this, isSaving/isDestroying aren't changing value currently
steal-stache upgrade
- The latest steal-stache isn't compatible with other prerelease libraries due to stateful dependencies. I temporarily replaced stache require()s in the tag test with fetch on module startup, but this should be reverted when steal-stache gets an upgrade.
possible problems with model store
- I made a standalone memory connection in can-fixture to eliminate the circular dependency on can-connect. All tests in can-fixture pass but some model store tests in can-connect fail, so this may be due to not adding proper instance references.
Other miscellaneous tests failing

justinbmeyer commented 6 years ago

We can drop the model “polyfill”

justinbmeyer commented 6 years ago

There may be a problem using canReflect.isBound(obj) ( issue )and related APIs for replacing the old can-connect/map behavior that integrates with can-connect/constructor/store.

In short, we were storing instances whenever they were bound to, not when they ran through the getList|get|create|update|destroy methods. For example:

Todo.get({id:1}).then(function(todo){
  setTimeout(() => {
    todo.on("name", handler) //-> put in store
  },10);
})

Instances without ids would be put in the newInstanceStore:

todo = new Todo();

todo.on("something", ...)

I see three courses of action:

make `can-reflect` or something similar be a global (and stateful) way of knowing this information

We could support something like:

canReflect.onBindChange(function(obj, isBound){
  if(obj === connection.Map && isBound) { PUT_IN_STORE(obj) }
})

Every observable would know to call this onBindChange method when its bound status changes.

`canReflect.onBindChange(Type, handler(obj, isBound) )`

Support .onBindChange on the individual types:

Todo = DefineMap.extend({ .. })

canReflect.onBindChange( Todo, function(todo, isBound) {} );

Pros:

Each type would have this state. So better.

Cons:

If we do observe(), we don't have a Type to listen to. Perhaps we decorate Object and Array so something like the following is possible:
```
canReflect.onBindChange( Object, function(todoObj, isBound) {} );
```
Even if someone wants to use POJO objects with can-connect, we probably need to Type them with something on their proto chain. This is so we could distinguish between something retrieved by can-connect and a real POJO.
```
class Todo() {
// does `observe(this)` automatically setup `Todo` for dispatching `.onBindChange`?
constructor(){ observe(this) }
}
```
What about inheritance? If Mammal inherits from Animal, does binding on a Mammal fire a Animal.onBindChange?

Do nothing for 4.0, figure it out after

We could release 4.0 working the old way, with only DefineMap and Map. Then we could figure out the right way of making it work later and try to squeeze in 4.0.

Only add to the store while going through can-connect instance methods

We could only "notice" bound elements if they bind immediately upon receiving an instance.

I don't think this will work as there's lots of behavior that can prevent immediate binding.

canjs / can-connect