.attr() has different behavior after can-reflect

[justinbmeyer]

Related to: https://gitter.im/canjs/canjs?at=5b574c6ef9ffc4664bfc5eee

After the can-reflect changes, map.attr() changed to recursively "unwrap" nested instances.

For example, it will attempt to unwrap instances of MyClass:

class MyClass {
  constructor(val1){
    this.val1 = val1;
  }
}

myMap = new can.Map({
  myClass: new MyClass(5)
});

myMap.attr().myClass !== myMap.attr("myClass");

After can-reflect, myMap.attr().myClass will be a POJO that looks like {val1: 5}.

Before can-reflect, myMap.attr().myClass would have been the actual instance.

This changed because can-reflect will always try to serialize (or unwrap) any objects it doesn't understand:

this.eachKey(value, function (childValue, prop) {
  serialized[prop] = this[methodName](childValue);
}, this);

Previously, can-map, was only recursively serializing (or unwrapping) isMapLike objects. That was done here: https://github.com/canjs/can-map/blob/v3.0.0/map-helpers.js#L81

Short term fix

The fix I suggested was to add the can.unwrap symbol to instances of MyClass so they unwrap to themselves:

class MyClass {
  constructor(val1){
    this.val1 = val1;
  }
}
MyClass.prototype[Symbol.for("can.unwrap")] = function(){
  return this;
}

Options

There's a few things we could to to fix this.

Decide what the right behavior is

First, we need to decide what the behavior should be in CanJS 4.0 and 5.0. I see two options:

• Change the behavior to "leave instances of unknown types alone". IMO, something that would be "unknown" would not have any symbols that CanJS recognizes useful for serialization. Not only can.serialize and can.unwrap, but also can.getOwnKeys and such.
  • PROS
    • Leave things alone
  • CONS
    • Possibly a breaking change
• Leave the behavior as it is already in 4.0
  • Maybe unwrap should make these things POJOs. But I feel like that is really .serialize()'s job.
  • PROS
    • Doesn't break anything

Change the behavior

I'll think about this later.

Leave the behavior alone

• We need to update the migration guide to explain this for people.
• We need to figure out what to do in 3.0.
  • We could make it so .attr() doesn't use unwrap(). That feels weird. Would we break other people's code who have worked around this?
  • We could leave this breaking change in, but warn about it somehow? Make it clear on the docs that this was a breaking change without a MAJOR release?

First, we should make the current behavior explicit in CanJS 4.0 and 5.0. Though I think there's something to