Promote dojox.lang.oo.declare to dojo.declare

[James Burke]

dojo.lang.oo.declare has:

• fixes for speed/behavior, resolving super chains.
• allows anonymous "class" declarations (not requiring a string for the first argument)
• more configurable options on automatic super call chaining.

Want to provide this as the default dojo.declare.

[Eugene Lazutkin]

(In [20068]) lang.oo: simplifications and performance improvements across the board, !strict, refs #9862.

[Eugene Lazutkin]

Previous commits: [17126], [19482], [19483], [19487], [20032], [20036], [20037], [20038], [20041].

The rest (e.g., different algorithms and tweaks) is in my private repository.

[Eugene Lazutkin]

(In [20088]) lang.oo: cleaning up oo.declare() to our coding standards, the previous version is preserved as declare-x, !strict, refs #9862.

[Eugene Lazutkin]

(In [20089]) lang.oo: clean up and fixing the preamble bug, !strict, refs #9862, refs #9795.

[Eugene Lazutkin]

(In [20126]) declare: committing the new version with updated inline documentation, and getInherited(), !strict, refs #9862, refs #9795.

[bill]

The only failure I'm seeing in dijit is dijit/tests/layout/ContentPaneLayout.html. Haven't looked into what exactly is failing but it works before the switch over.

[Eugene Lazutkin]

Replying to bill:

The only failure I'm seeing in dijit is dijit/tests/layout/ContentPaneLayout.html. Haven't looked into what exactly is failing but it works before the switch over.

These are links to dojo.declare:

• 9/15 ​http://archive.dojotoolkit.org/dojo-2009-09-15/dojotoolkit/dojo/_base/
• 9/16 ​http://archive.dojotoolkit.org/dojo-2009-09-16/dojotoolkit/dojo/_base/
• 9/17 ​http://archive.dojotoolkit.org/dojo-2009-09-15/dojotoolkit/dojo/_base/

They all use old dojo.declare (you can inspect files).

These are links to the test in question:

• 9/15 ​http://archive.dojotoolkit.org/dojo-2009-09-15/dojotoolkit/dijit/tests/layout/ContentPaneLayout.html
• 9/16 ​http://archive.dojotoolkit.org/dojo-2009-09-16/dojotoolkit/dijit/tests/layout/ContentPaneLayout.html
• 9/17 ​http://archive.dojotoolkit.org/dojo-2009-09-17/dojotoolkit/dijit/tests/layout/ContentPaneLayout.html

The test from 9/15 works for me in FF, other two fail --- all three use old dojo.declare.

The test was changed on 9/15 and most probably at that time some bugs were introduced. Could you double-check that it works with the old dojo.declare? I cannot do it locally because the test requires PHP, and I don't have it on my web server.

[Eugene Lazutkin]

the old version of dojo.declare, can be used for testing and as a reference

[bill]

Yes, I used the old version of dojo.declare() as a reference, that's how I know that it's the new dojo.declare() where it breaks. Even triple checked. I'm not sure what's up with the nightlies, perhaps just a timing race condition. Anyway the latest code works for me only if I rollback [20126].

The first failing test is "single resizable href", and the problem is that StackContainer.resize() calls this.inherited() which calls _LayoutWidget.resize(). There's a connection to that _LayoutWidget.resize() call which isn't getting called:

dojo.connect(dijit.layout._LayoutWidget.prototype, "resize", function(){
	// this is the pointer to the widget, and arguments are newsize/curSize args to resize()
	var ary = layoutResizes[this.id];
	if(!ary){
		ary = layoutResizes[this.id] = [];
	}
	ary.push(arguments);
});

I don't know if you want to support connecting to prototype methods, but it was working before and I remember some other code somewhere that connects to methods on the prototype.

[dante]

​http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/dtl/demos/demo_DomTemplated.html is also broken.

[Eugene Lazutkin]

(In [20154]) dtl: minor fix --- "classes" should be functions, not objects, !strict, refs #9862.

[Eugene Lazutkin]

(In [20155]) dojo.declare: minor refactoring, support for dynamically redefined methods, !strict, refs #9862.

[Eugene Lazutkin]

Replying to bill:

[20155] should take care of dojo.connect on prototype methods problem. I can't test it myself, so please try to run it and see if it works for you.

Replying to dante:

[20156] should take care of the DTL bug. One of "classes" was an object with a prototype property. It worked before because the old version of dojo.declare didn't allow native objects. New version assumes that if "class" doesn't define the "constructor" property in the prototype, that the "class" itself (a function) is used to construct the object. Obviously it doesn't work with plain objects. The fix is trivial.

[Eugene Lazutkin]

Sorry, [20156] is wrong. It should be [20154].

[Eugene Lazutkin]

(In [20156]) dojo.declare: new test that uses 3 different forms of prototype method augmentation/replacement, !strict, refs #9862.

[Eugene Lazutkin]

[20156] emulates Bill's case and it works now. Still we need to test the real thing.

[Eugene Lazutkin]

(In [20157]) lang.oo: temporarily switching oo.declare tests to the new dojo.declare to verify that it works as expected, !strict, refs #9862.

[bill]

Cool, ContentPaneLayout.html is working now, thanks.

[Eugene Lazutkin]

(In [20158]) dojo.declare: more accurate work with inherited "raw classes", !strict, refs #9862.

[Eugene Lazutkin]

(In [20160]) lang.oo: modified the benchmark to use the existing (new) dojo.declare), the old one is copied and modified to make this benchmark possible, !strict, refs #9862.

[Eugene Lazutkin]

(In [20161]) declare: minor code reduction, !strict, refs #9862.

[Eugene Lazutkin]

(In [20162]) dojo.declare: faster algorithm for C3 MRO, !strict, refs #9862.

[Eugene Lazutkin]

(In [20163]) dojo.declare: a speed bust, !strict, refs #9862.

[Les]

or rather speed boost :)

[bill]

Could you check dijit/tests/_Templated-widgetsInTemplate.html? I'm getting a hang in the c3mro code. Thanks.

[Eugene Lazutkin]

Replying to Les:

or rather speed boost :)

:-)

Replying to Bill:

Could you check dijit/tests/_Templated-widgetsInTemplate.html? I'm getting a hang in the c3mro code. Thanks.

Will do.

[Eugene Lazutkin]

Replying to bill:

Could you check dijit/tests/_Templated-widgetsInTemplate.html? I'm getting a hang in the c3mro code. Thanks.

That test code inadvertently triggered a bug in my code. Here is how TestLayoutWidget is defined:

dojo.declare('TestLayoutWidget',
  [dijit._Widget, dijit.layout._LayoutWidget], {
...
});

Note dijit._Widget as the base, and dijit.layout._LayoutWidget as the mixin.

Here is how dijit.layout._LayoutWidget is defined:

dojo.declare("dijit.layout._LayoutWidget",
  [dijit._Widget, dijit._Container, dijit._Contained], {
...
});

As you can see the dijit._Widget is here again. While this structure is perfectly legal, it doesn't make much sense.

With the old dojo.declare dijit._Widget is going to be doubled. With the new one it is supposed to be collapsed, but I had a bug in my code.

I'll check in the fix shortly after running more tests to make sure I didn't break anything else while fixing the bug.

[bill]

Ah, ok thanks! I'll fix that error in _Templated-widgetsInTemplate.html (it wasn't intended to include _Widget twice).

[bill]

(In [20173]) TestLayoutWidget? (unintentionally) extended from _Widget twice. Refs #9862.

[Eugene Lazutkin]

Replying to bill:

(In [20173]) TestLayoutWidget? (unintentionally) extended from _Widget twice. Refs #9862.

Another possible enhancement (does not matter in this particular case of course) is to change the code from this pattern:

declare("A", [B], {...});

to this:

declare("A", B, {...});

because it is a tiny bit faster --- it is handled as a special case.

Another tiny enhancement is to convert this code:

declare("A", [], {...});

to this:

declare("A", null, {...});

It is a special case too.

[Eugene Lazutkin]

(In [20174]) dojo.declare: minor refactoring, !strict, refs #9862.

[Eugene Lazutkin]

(In [20175]) Fixing the bug reported by Bill, switching to arrays for C3 MRO algorithm, minor speed tweaks, !strict, refs #9862.

[Nathan Toone]

The following code used to work previously (I saw it on some example website at one point) but it now fails with the error "Mixin #0 is null":

dojo.declare("A", [null, B], {...});

The idea being you want to create a "double-mixin" - you don't want it to inherit directly from any class - but you want it to have another mixin's class mixed in.

[Nathan Toone]

Another issue that is not working with these changes (but did work previously):

dojo.setObject("MyMixin"), {
  resize: function(){
    this.inherited("resize", arguments);
  }
}
var cp = new dijit.layout.ContentPane();
dojo.mixin(cp, MyMixin);
cp.resize();  // <- This gives an error because it can't find the "inherited" function to call

Again - this was based off an example that I had read somewhere - and it has worked until these latest changes.

[Nathan Toone]

correction: the resize() call in the above example doesn't give an error - it just doesn't call the inherited function.

[Eugene Lazutkin]

Replying to toonetown:

The following code used to work previously (I saw it on some example website at one point) but it now fails with the error "Mixin #0 is null":

That one should be changed in your code --- one more special case will bloat the code more (I already added almost 100 bytes to support inheritance from [] instead of null and [A] instead of A). Frankly I don't see any compelling reasons for "double-mixin".

Even if you do have a highly compelling reason for that (I can't see any), you can always do:

var A = function(){};
dojo.extend(A, B.prototype, {...});

[Eugene Lazutkin]

Replying to toonetown:

Another issue that is not working with these changes (but did work previously):

While general dynamic mixing of methods is not supported directly (they don't have a proper meta-information for one), your example should work. Probably there is a bug in my code. I'll look into that.

[Nathan Toone]

Replying to elazutkin:

That one should be changed in your code --- one more special case will bloat the code more (I already added almost 100 bytes to support inheritance from [] instead of null and [A] instead of A). Frankly I don't see any compelling reasons for "double-mixin".

OK - I'm fine with that as a solution.

[Nathan Toone]

Replying to elazutkin:

While general dynamic mixing of methods is not supported directly (they don't have a proper meta-information for one), your example should work. Probably there is a bug in my code. I'll look into that.

I can write up a test case, if you like.

[Eugene Lazutkin]

Replying to toonetown:

I can write up a test case, if you like.

The current code handles two dynamic things:

1. One of the prototypes is modified dynamically, and the new method calls this.inherited() directly. Obviously it needs to supply a name.
2. One of the prototypes is modified dynamically, and the new method calls the old method, which may call this.inherited() (as it was programmed to do).

Apparently I missed the addition of a dynamic method directly on an instance. I'll add the support for it and it will work like this: it will call the first inherited method available effectively starting the chain of inherited calls from the beginning. If this is something you expect, I'll write a unit test myself, otherwise you should write it and explain how you expect it to work.

[Nathan Toone]

Replying to elazutkin:

Apparently I missed the addition of a dynamic method directly on an instance. I'll add the support for it and it will work like this: it will call the first inherited method available effectively starting the chain of inherited calls from the beginning. If this is something you expect, I'll write a unit test myself, otherwise you should write it and explain how you expect it to work.

Yes - that is what I would expect. Basically, just being able to mix it into an existing instance.

[bill]

(In [20195]) Don't use array when there's only one superclass. It's a tiny bit faster (and two less bytes of code...). Refs #9862 !strict.

[Nathan Toone]

(In [20203]) Refs #9862 - add test case for mixing in new functions to an instance of a class - this test passes with revision [20154] (though the mutatedMethods test breaks in that revision) and breaks in [20155] (which fixes the mutatedMethods test) !strict

[Eugene Lazutkin]

(In [20214]) dojo.declare: fixing a bug reported by toonetown, thx!, !strict, refs #9862.

[sjmiles]

As discussed on dojo-dev, old declare added an 'extend' to function to declared constructors that added methods to a prototype and added helpful meta-data. Tried to make a patch to restore that functionality, but ran into this problem:

dojo.declare("A", null, {});

// simulate A.extend
A.prototype.id = function() {
		log("A.id");
};
A.prototype.id.nom = "id";
		
dojo.declare("B", A, {
	id: function() {
		log("B.id");
		this.inherited(arguments);
	}
});
		
b = new B();
b.id(); // outputs B.id but *not* A.id also, as expected

There are other dynamic extension cases that fail, but they may either be (1) homeomorphic with this case or (2) too esoteric to worry about. I will follow up after we decide what to do about the above.

Will attach test file. Please let me know if I messed up.

[sjmiles]

Test case for dynamic (post-declare) class extension

[Eugene Lazutkin]

(In [20240]) dojo.declare: minor clean up, renaming, and speeding up, !strict, refs #9862.

[Eugene Lazutkin]

(In [20241]) dojo.declare: different way to produce empty functions closure-free, !strict, refs #9862.

[Eugene Lazutkin]

Replying to sjmiles:

... but ran into this problem:

Both your examples work for me. Please double-check that you use the recent trunk. If everything is ok, but the problem is there, can you host it somewhere so I can access it, yet you still see the problem? You should have an account at dojotoolkit.org to host the test.

[sjmiles]

Confirmed this is working against trunk, thank you.

It also seems that dojo.extend will simply work for declared classes now and we don't need any special class extend, correct? In that case I don't have any problem with removing that feature.

[sjmiles]

Replying to elazutkin:

(In [20241]) dojo.declare: different way to produce empty functions closure-free, !strict, refs #9862.

Well done. I'm still not sure if it matters, but now we don't have to worry about it.

[Eugene Lazutkin]

(In [20276]) dojo.declare: refactoring, size reduction, speed improvements, adding extend() like in the old dojo.declare(), !strict, refs #9862.

[Eugene Lazutkin]

(In [20277]) dojo.declare: more refactoring to conserve space, speed up, and process edge cases, !strict, refs #9862.

[Eugene Lazutkin]

(In [20278]) dojo.declare: more test cases with esoteric inheritance, !strict, refs #9862.

[Eugene Lazutkin]

(In [20279]) dojo.declare: removing extra spaces, !strict, refs #9862.

[Eugene Lazutkin]

(In [20284]) dojo.declare: speed up and size reduction, !strict, refs #9862.

[Eugene Lazutkin]

(In [20315]) dojo.declare: more tests, !strict, refs #9862.

[Eugene Lazutkin]

(In [20525]) dojo.declare: updating the benchmark and the tests for new interface, !strict, refs #9862.

[Eugene Lazutkin]

Missing commit: [20524].

[Eugene Lazutkin]

Missing commit: [20527].

[bill]

(In [20616]) Remove vestigal comment, refs #9862 !strict.

[bill]

See also #10169 and #10170, which were broken by the [20284] commit to this ticket.

[Eugene Lazutkin]

(In [20700]) declare: adding immediate parents to the meta information, !strict, refs #9862.

[Eugene Lazutkin]

Added dojo.declare and dojo.safeMixin to the 1.4 release nodes, updated dojo.declare docs on Dojo Campus (1st draft).

[Eugene Lazutkin]

(In [20738]) dojo.declare: performance improvements, minor API updates, !strict, refs #9862.

[Eugene Lazutkin]

(In [20739]) dojo.declare: updating the benchmark, !strict, refs #9862.

[bill]

[20738] is broken, in dijit/tests/test_TitlePane.html on example 3, when you open the pane the TabContainer is not visible. (Found this from the automated regression tests.) Probably it's another problem with this.inherited() not hitting all the classes but I didn't trace it down.

[Eugene Lazutkin]

The last change breaks #10300.

[Eugene Lazutkin]

(In [20755]) dojo.declare: regression cleanup, !strict, fixes #10300, refs #9862.

[Eugene Lazutkin]

Replying to bill:

I think I fixed it now --- all tests are good, and both reported cases work as expected (thx to Doug and you), but please double-check that.

[bill]

Unfortunately it's still broken (although TitlePane is working). Using dojo.declare() from [20755] on IE8, dijit/tests/layout/ContentPaneLayout.html gets 4 failures. It works when using dojo.declare() from [20700]

[Eugene Lazutkin]

(In [20791]) dojo.declare: fixing the kinks reported by Bill (thx!), !strict, refs #9862.

[Eugene Lazutkin]

(In [20796]) dojo.declare: inlining and some code reduction, !strict, refs #9862.

[Eugene Lazutkin]

(In [20797]) dojo.declare: removing unnecessary assignments, !strict, refs #9862.

[Eugene Lazutkin]

(In [20798]) dojo.declare: cleanup, !strict, refs #9862.

[Eugene Lazutkin]

(In [20799]) dojo.declare: making "constructor" a named variable, reformulating inherited() to gain back performance on Chrome, !strict, refs #9862.

[Eugene Lazutkin]

(In [20800]) dojo.declare: eliminating trailing spaces, !strict, refs #9862.

[Eugene Lazutkin]

(In [20801]) dojo.declare: minor tweaks to tests and benchmarks, !strict, refs #9862.

[Eugene Lazutkin]

Amazingly adding a single dictionary lookup made inherited() on Chrome/V8 6 (six) times slower:

var op = Object.prototype;
// ...
var opf = op[name]; // THIS ONE! NOT IN THE LOOP!

I can't believe it is so slow. If you look at inherited() it is a relatively large function with dozens operations. Yet, opf overweighted them all. I suspect that Object.prototype is implemented differently than normal JavaScript objects, because all other lookups are very fast.

[bill]

These recent changes break dijit again. The button to open a menu for a CombButton (see Save button in test_Button.html) doesn't open the menu. It works in [20791] but fails in [20800].

[Eugene Lazutkin]

(In [20810]) dojo.declare: fixing another gotcha by Bill (thx!), !strict, refs #9862.

[Eugene Lazutkin]

All found problems and enhancements should go in separate tickets.

[Eugene Lazutkin]

(In [20880]) dojox.lang.oo: removing unneeded files, which were promoted to dojo.declare, !strict, refs #9862.