Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16554 closed defect (invalid)

inherited calls function on "this.prototype" instead of superClass

Reported by: mm Owned by:
Priority: undecided Milestone: tbd
Component: General Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description

	var stack = [];
	var P = declare(null, {
		m0 : function() {
			stack.push("P.m0");
		}
	});
	var CH = declare(P, {
		m0 : function() {
			stack.push("CH.m0");
		},
		m4 : function() {
			stack.push("CH.m4");
			this.inherited("m0", arguments);
		}

	});

	return {

		"inheritedWidthDifferentNameOnChildAsWell" : function() {
			stack = [];
			var c1 = new CH();
			c1.m4(); //!!!
			doh.is(stack, answers.inheritedWidthDifferentNameOnChildAsWell(), "inheritedWidthDifferentNameOnChildAsWell");
		}

	};

I expect: CH.m4,P.m0 but in reallity it returns CH.m4,CH.m0

It seems that it is not possible to call inherited method from parent if the method exists on child (CH), and if this method is called from differently named method.

If method m0 does not exists och child, it works correctly and calls P.m0 not CH.m0.

I have not found any explanation for this.

Please review the code, and include n test cases if possible, with explanation, please (if this is really intended feature)

Change History (7)

comment:1 Changed 6 years ago by Eugene Lazutkin

Resolution: invalid
Status: newclosed

this.inherited() is a facility to call method's super, not a generic way to manipulate an inheritance chain. It cannot be used to call arbitrary methods from a single call site. Moreover, I believe calls with signature like this.inherited("m0", arguments) are not documented, and generally not supported outside of a very specific use case, which doesn't apply here.

comment:2 Changed 6 years ago by mm

RE: I believe calls with signature like this.inherited("m0", arguments) are not documented.

It is documented: however formulation "should be" is IMHO not satisfactory.



getInherited: function(name, args){
			// summary:
			//		Returns a super method.
			// name: String?
			//		The optional method name. Should be the same as the caller's
			//		name. Usually "name" is specified in complex dynamic cases, when
			//		the calling method was dynamically added, undecorated by
			//		declare(), and it cannot be determined.

BTW: I do not see any reason in the code, why this shall not be supported. Please enlighten me. Also the docs shall supply some information how to achive this usuall behavior. (calling P.m1 from CH.m2 with dojo.declare) Thanx

comment:3 in reply to:  2 Changed 6 years ago by Eugene Lazutkin

Replying to mm:

It is documented: however formulation "should be" is IMHO not satisfactory.

Let's read the documentation cited by you:

The optional method name. Should be the same as the caller's name.

Let me stress: not arbitrary name, but the caller's name.

Continuing:

Usually "name" is specified in complex dynamic cases, when the calling method was dynamically added, undecorated by declare(), and it cannot be determined.

It means literally what it means. This code wouldn't work, because the added method function is undecorated (has no meta-data):

abc.xyz = function(){ return this.inherited(arguments); };

In order to point inherited() to the right chain, the name should be added:

abc.xyz = function(){ return this.inherited("xyz", arguments); };

Obviously it works only for top-level methods, which is usually the case.

BTW: I do not see any reason in the code, why this shall not be supported.

It is just not. It is not a bug. This functionality is explicitly not supported.

Please enlighten me. Also the docs shall supply some information how to achive this usuall behavior. (calling P.m1 from CH.m2 with dojo.declare) Thanx

dojo.declare() doesn't have provisions to call arbitrary methods of a dynamic superclass. JavaScript doesn't support it in any way nor fashion, and dojo.declare() was not engineered to support this functionality either.

While this functionality may be useful in certain scenarios, it is quite exotic, and this ticket is an evidence of that: I believe it is the first such request in all history of Dojo (8 years).

comment:4 Changed 6 years ago by mm

calling super function from another function is not exotic. Manyof JS frameworks support it by calling syntax:

methodA: function(){return ...;...;superclass._methodB()}; specially if _methodB is protected.... etc... it is normal (if class based inheritance can be called normal at all)

yes it is exotic (to dojo people): getInhetired is not used in dojo, dijit, dojox and have 0 testcases (even if it is public, not underscored not double underscored and documented). Same applies for borde testcases of inherited(name,...) signatures.

I;m done with this topic (as bug) and may discuss somewhere else...

Just fix docs, testcases (and looking at the code, this one needs about 3,4 ifs, or deleting some ++pos to work as well). Thanx.

Behold GIGO and prototype inheritance ;-) Thanx as well.

comment:5 Changed 6 years ago by bill

If you really wanted to be pedantic you could change the API doc from saying:

Should be the same as the caller's name.

to

Must be the same as the caller's name.

comment:6 Changed 6 years ago by mm

thank you bill for support :-)) including sentence: if you dont you will not get exception or undefined or other semi-resonable value but

your own method today or someting else tomorrow ;-)))

sorry made me laug:-) i love border conditions and docs of dojo, really

comment:7 Changed 6 years ago by Eugene Lazutkin

:-)

I peppered staging docs with MUST. Hopefully people will take it the right way: GIGO.

Note: See TracTickets for help on using tickets.