UselessPickles commented 8 years ago

The define plugin does not seem to properly support inheritance as documented for version 2.3: https://canjs.com/docs/can.Map.prototype.define.html#section_Inheritance

See this jsfiddle for a demonstration: http://jsfiddle.net/gqrjdb53/11/

There is a ViewModelBase class that has a synthetic property defined: "messageToDisplay". It should evaluate to either the value of "niceMessage" or "meanMessage", depending on the state of the "beMean" boolean field.

The key part of the view template is: {{messageToShow}} {{name.first}} {{name.last}}

The "beMean" field is bound to the checkbox, so clicking the checkbox should toggle between "Hello Bob Smith" and "Go Away Bob Smith".

However, the synthetic "messageToShow" property seems to never be properly initialized. It doesn't seem to be properly inherited by the ViewModel class.

When stepping through the code for the ViewModel constructor, I noticed that within the overridden implementation of _setupComputedProperties (the "define" plugin's implementation), "this.define" contains only the define properties of the child class. The parent class "define" does not appear to ever be processed.

If inheritance is not used, then the synthetic "messageToShow" property works as expected. There is a second (commented-out) definition of ViewModel that does not rely on inheritance. When this alternate definition is uncommented, the demo works as expected.

UselessPickles commented 8 years ago

Looks like there's a pull request that has been open since early this year: https://github.com/canjs/canjs/pull/2222

Has this been forgotten?

justinbmeyer commented 8 years ago

Is this not solved by https://github.com/canjs/can-map-define/issues/2 ?

UselessPickles commented 8 years ago

2 Is closed, but the pull request into master is still open. The latest release version (2.3.27) does not yet implement support for inheritance (demonstrated in the jsfiddle), but the public documentation claims that it does.

pYr0x commented 8 years ago

@UselessPickles I think this issue is solved only in canjs 3.0 and only in the splited out module can-map-define http://canjs.github.io/canjs/doc/can-map-define.html

https://github.com/canjs/can-map-define/pull/13 Was the PR for that

UselessPickles commented 8 years ago

@pYr0x I am aware that it is currently only fixed in 3.0. That's why I am reporting this issue (for version 2.3). The documentation for 2.3 clearly specifies that inheritance is supported: https://canjs.com/docs/can.Map.prototype.define.html#section_Inheritance

But the latest version of 2.3 does not yet support inheritance.

Is there any plan to add inheritance support to 2.3? Has the documentation been prematurely updated with version 3.0-specific features? The documentation does not match the functionality, so one or the other needs to be updated to match.

justinbmeyer commented 7 years ago

I think the documentation was prematurely updated. We'll happily accept a pull request into 2.3, but this isn't priority right now.

canjs / can-map-define

Defined properties are not inherited properly #17

2 Is closed, but the pull request into master is still open. The latest release version (2.3.27) does not yet implement support for inheritance (demonstrated in the jsfiddle), but the public documentation claims that it does.