These are chat archives for canjs/canjs

31st
Jul 2018
RyanMilligan
@RyanMilligan
Jul 31 2018 00:22
I went ahead and logged the issue here: canjs/can-construct#62
can-construct was a bit of a guess, it's hard to say exactly where the problem is. Much of the code involved is in can-define, but it seems to be specific to the way can-construct works with prototypes.
Justin Meyer
@justinbmeyer
Jul 31 2018 01:49
@/all Just an FYI, I'm out for 2 weeks giving a training. Chasen is on client work. Matthew is starting some client work next week. This leaves only Kevin to help out this and next week.
This means that we'll have less of the "core team" available this and next week to help answer questions.
Hopefully, other folks in the community can assist :-)
Justin Meyer
@justinbmeyer
Jul 31 2018 01:58
@RyanMilligan thanks for reporting, moved the issue to can-define ... I have some questions to better understand the context in which this is happening: canjs/can-define#364
Because reading from the prototype like Test.prototype.title isn't (and mostly "can't" be) supported
If you need to do that, it should be done like Object.getOwnPropertyDescriptor(Test.prototype,"title")
RyanMilligan
@RyanMilligan
Jul 31 2018 15:35
@justinbmeyer I'm actually not sure exactly where that's happening; we're certainly not doing it on purpose. Now that I know what's triggering it, I plan to search the code and see if I can figure that part out. All I know at the moment is that we were seeing issues where the prototype was changing seemingly randomly and causing values from one object to be applied to unrelated objects being created later, and this is what I traced the issue down to.
Justin Meyer
@justinbmeyer
Jul 31 2018 15:36
If you know the property
that is breaking, you could change to a default function
and set a debugger
DefineMap.extend({
  myProp: {
    default(){
     debugger;
     return "value";
    }
  }
})
that debugger should hit when the odd behavior is happening
RyanMilligan
@RyanMilligan
Jul 31 2018 15:38
Will that work in Can 3?
Justin Meyer
@justinbmeyer
Jul 31 2018 15:38
use value instead of default and it should
RyanMilligan
@RyanMilligan
Jul 31 2018 15:41
Ok, I'll keep that in mind. Right now I'm just searching for references to prototypes.
I found a place in can-stache-converters.js that I'm pretty sure would trigger this, FYI. The isConstructor helper loops through func.prototype, which should cause this issue on the first key in the prototype.
        for(let prop  in func.prototype) {
            return true;
        }
Never mind, that's something my predecessor copied from somewhere. Not sure if that's in the "real" code.
Kevin Phillips
@phillipskevin
Jul 31 2018 15:54
yeah, I don't think that's really in can-stache-converters
RyanMilligan
@RyanMilligan
Jul 31 2018 15:55
Yeah, I didn't find it either. Oh, there's a comment at the top that indicates it's the Can v2 version, so maybe it's been removed since.
RyanMilligan
@RyanMilligan
Jul 31 2018 16:10
Found it! Looks like the impact is a bit wider than I thought. The title property was being broken in this way by a piece of framework code accessing the tag property on the same object. I thought it was only the specific property being accessed that caused it, but it looks like it might be accessing any property.
RyanMilligan
@RyanMilligan
Jul 31 2018 17:21
Hrm, @phillipskevin, it looks like getOwnPropertyDescriptor() isn't working as well as I thought. First of all, the actual code needed is Object.getOwnPropertyDescriptor(MyType.prototype, 'property').get.apply(MyType.prototype), which is pretty unwieldy. I can put it in a helper function, though.
The bigger problem is that the get function is actually the same get function that's getting called when you access the prototype directly, so I'm pretty sure it will have the same issue. I think I'm actually just going to use MyType.instance().property for now.
Kevin Phillips
@phillipskevin
Jul 31 2018 17:24
where are you using this?
can't you just read property from the instance?
RyanMilligan
@RyanMilligan
Jul 31 2018 17:25
You know, maybe the property I'm using here should just be a static property. That would probably be easier.
I have a class with several derived variants that each set a different value for a property called tag, which I then use to search a list of instances for. So my function takes a tree of instances and a type, which it gets the tag from. This seemed reasonable at the time because I didn't know accessing the prototype directly was unsafe, but I should probably just do it a different way now that I do.
Kevin Phillips
@phillipskevin
Jul 31 2018 17:29
ok, if that doesn't work out let us know
RyanMilligan
@RyanMilligan
Jul 31 2018 17:33
Will do. I still think it would be good to get some sort of warning on that, since there's really no way to know it's not supported until everything goes completely haywire.
RyanMilligan
@RyanMilligan
Jul 31 2018 17:54

@phillipskevin Quick question for you: I'm trying to change the default list and map types that are used by the observable type, and it looks like there are two separate properties to set on can-types to do so, only one of which is actually defined. Is this expected? Here are the two types that use them, from can-define.js:

    'observable': function(newVal) {
                if(Array.isArray(newVal) && types.DefineList) {
                        newVal = new types.DefineList(newVal);
                }
                else if(isPlainObject(newVal) &&  types.DefineMap) {
                        newVal = new types.DefineMap(newVal);
                }
                return newVal;
        },
    'stringOrObservable': function(newVal) {
        if(Array.isArray(newVal)) {
            return new types.DefaultList(newVal);
        }
        else if(isPlainObject(newVal)) {
            return new types.DefaultMap(newVal);
        }
        else {
            return define.types.string(newVal);
        }
    },

Are there supposed to be separate DefaultList\DefaultMap and DefineList\DefineMap properties to override?

Kevin Phillips
@phillipskevin
Jul 31 2018 19:02
@RyanMilligan what version of can-define are you using?
RyanMilligan
@RyanMilligan
Jul 31 2018 20:00
I'm using 1.5.6 (because we're still using Can 3), but I see the same thing in the master branch on GitHub: https://github.com/canjs/can-define/blob/8aa77111a5542dabd9223d9ff36004073e6dda32/can-define.js#L1101.