Mats Bryntse
31 January 2013

Road to Ext Scheduler v2.2: Overrides

We’re currently busy finalizing a major refactoring of our Ext Scheduler component which will be released as v2.2. There are […]

We’re currently busy finalizing a major refactoring of our Ext Scheduler component which will be released as v2.2. There are two major reasons for this refactoring. First of all we needed to refactor to enable certain pieces of the core functionality to be shared with our Touch Scheduler. These bits of code relate to non-UI parts of the component, such as stores, models, utility classes etc. The second reason for the refactoring, was to future proof the component a bit and clean up as many Ext JS overrides as possible. With the recent Ext JS 4.2 beta releases, too many things broke that should not break.

Global Ext JS overrides

In the latest official version of Ext Scheduler, things are pretty much under control. There’s a large unit test suite covering loads of scenarios and the component is very stable. For example, we’re testing that no global variables are leaked. We also scan the entire Ext JS source tree to find any global overrides of the Ext JS library (a bad practice we used in older versions).

[crayon striped=”false” lang=”javascript” nums=”false” toolbar=”false”]
// Our Siesta suite will detect this and fail
Ext.grid.View.override({
refresh : function() {
// Do something else
}
});
[/crayon]

We don’t write code like this anymore. Override code like this introduces a lot of uncertainty and is just as bad as regular global variables. As a component maker, we should not make any changes to our ‘surroundings’. Thanks to our zero-tolerance test, any attempt to make a global Ext JS override will now break our build. You can check how the test is performed in our test named /tests/sanity/012_no_overrides.t.js .

Overriding private Ext JS methods

Another aspect of overrides is when you override a private method in a subclass (see code below). When faced with a tricky problem, this can be tempting since there is nothing stopping you in the world of javascript and Ext JS. But overriding non-public and non-documented methods will likely lead to problems, maybe not tomorrow – but at some point. It happened to us quite frequently with each new version of Ext JS we had to support. At too many places we had simply overwritten private Ext JS methods, whose implementations then changed slightly in newer releases.

[crayon striped=”false” lang=”javascript” nums=”false” toolbar=”false”]
// Overriding a private method of the superclass
Ext.define(“Sch.mixin.TimelineGridView”, {
extend : ‘Ext.grid.View’,

// We don’t want Ext guessing if this row should be repainted or not
// @OVERRIDE
shouldUpdateCell : function() { return true; }
});
[/crayon]

The snippet above looks harmless but it could very well lead to an issue if the shouldUpdateCell method is renamed or removed from the Ext GridView class. Overrides like this should be considered as the last resort to solve a particular problem. If there is a way to solve the issue by relying on the public Ext JS API instead it is definitely preferred. In real life applications, it’s hard to completely avoid doing such overrides though, so how do you best deal with these overrides? We already tried ignoring the overrides, and it turns out that ignoring a problem doesn’t really solve it. 🙂

Dealing with overrides

In our upcoming 2.2 version, each override of a private method is annotated with a // @OVERRIDE comment to warn anyone reading the code. If unit tests start breaking when we try a new version of Ext JS, those methods are prime candidates to review. This however is not enough, we can do better. I just wrote a simple Siesta unit test that will help us identify our weak spots as we upgrade to newer versions of Ext JS. It turned out to be very easy:

[crayon striped=”false” lang=”javascript” nums=”false” toolbar=”false”]
StartTest(function (t) {
t.expectGlobal(‘Docs’); // JsDuck

// Ignore some symbols that should be ignore + some bugs in the Ext docs
var ignoreRe = /Ext.dd|DragZone.destroy|DragDrop.destroy|DragSource.destroy|Ext.grid.plugin.Editing.init|afterRender|initComponent|Ext.Base.configClass|Ext.Base.destroy/;

var isPrivate = function(fullName) {
var priv = false;

Ext.Array.each(Docs.data.search, function(property) {

if (property.fullName === fullName){
priv = property.meta.private;
return false;
}
});

return priv;
};

function findInSuperClasses(sourceCls, property) {
var cls = sourceCls.superclass.self;

while (cls && cls.prototype) {
var name = Ext.ClassManager.getName(cls);
var fullName = name + ‘.’ + property;

if (name.match(/^Ext./) &&
!ignoreRe.test(fullName) &&
cls.prototype.hasOwnProperty(property))
{
if (isPrivate(fullName)) {
return name;
} else {
return false;
}
}
cls = cls.superclass && cls.superclass.self;
}

return false;
}

var MAX_NBR_OVERRIDES = 10;
var nbrFound = 0;

Ext.iterate(Ext.ClassManager.classes, function (className, constr) {
if (!className.match(‘Sch.’)) return;

for (var o in constr.prototype) {

// Check only own properties, and only functions for now
if (constr.prototype.hasOwnProperty(o) && typeof constr.prototype[o] === ‘function’) {
var result = findInSuperClasses(constr, o);

if (result) {
t.todo(function(t) {
t.fail(‘Class ‘ + className + ‘ overrides ‘ + result + ‘:’ + o);
})
nbrFound++;
}
}
}
});

t.isLessOrEqual(nbrFound, MAX_NBR_OVERRIDES, ‘Should not introduce new overrides of private methods’);
});
[/crayon]

The test isn’t 100% fool proof, you could still override classes at run time that won’t be detected by this test (though that is a very unusual practice). Running this test against the latest official Ext Scheduler release reports 65 overrides, which is waaaay too many. This is very clear proof and it explains why we’ve been experiencing painful upgrades. After our 2.2 refactorings, the result looks a lot better:

[crayon striped=”false” lang=”javascript” nums=”false” toolbar=”false”]
Launching PhantomJS 1.6.0 at https://lh/ExtScheduler/tests/index-no-ui.html

fail 1 – Class Sch.mixin.Lockable overrides Ext.grid.Lockable:constructLockablePlugins
fail 2 – Class Sch.mixin.Lockable overrides Ext.grid.Lockable:injectLockable
fail 3 – Class Sch.model.Customizable overrides Ext.data.Model:afterEdit
fail 4 – Class Sch.data.FilterableNodeStore overrides Ext.data.NodeStore:onNodeExpand
fail 5 – Class Sch.selection.EventModel overrides Ext.selection.Model:bindComponent
fail 6 – Class Sch.selection.EventModel overrides Ext.selection.Model:onSelectChange
fail 7 – Class Sch.plugin.TreeCellEditing overrides Ext.grid.plugin.CellEditing:onEditComplete
fail 8 – Class Sch.view.TimelineGridView overrides Ext.view.Table:shouldUpdateCell
fail 9 – Class Sch.view.TimelineGridView overrides Ext.view.View:processUIEvent

[FAIL] sanity/017_override_scan.t.js?Ext=external
There are failures
[/crayon]

We have now brought it down to 9 overrides, which is a lot more manageable. For an extremely small investment (the test took me < 1hr to write) we now have a very good overview of our component weak spots when it's time for the next upgrade. As the 2.2 version relies a lot more on the public API, upgrading to and supporting new Ext JS versions should hopefully be much easier. How do you deal with overrides in your application? Please share any tips you have in the comments section?

Mats Bryntse

Ext Scheduler