Ext.applyIf() doen't work for empty class members

[hutzelknecht]

Ext.applyIf checks for "undefined" values by typed comparison and therefore ignores "null" valued members. When subclassing GeoExt classes the applyIf-method proved useful in the past. My suggestion would be to exchange all empty member declarations. Any possible negative side effects to this suggestion?

see: http://docs.sencha.com/extjs/4.2.1/source/Ext.html#Ext-method-applyIf

[marcjansen]

I have a hard time to understand what you are actually suggesting. Can you possible give an example (preferable with some before/after code). Thanks.

[marcjansen]

I didn't want to close this, sorry.

[hutzelknecht]

Consider the following example:

Ext.define('myproject.view.Map', {
    extend: 'GeoExt.panel.Map',

    requires: [
        'myproject.view.MapToolbar'
    ],

    xtype: 'myproject-view-map',

    layers: undefined, // by overriding the parent class member with undefined instead of null the statement down below will work

    constructor: function()
    {
        this.addEvents(
            'someevent'
        );

        this.callParent( arguments );
    },

    initComponent: function() {
        var me = this;

        Ext.applyIf(me, {
            // These
              enableDrag: true
            , enableDrop: true
            , ddGroup: 'sensorDDGroup'
            , dropGroup: 'sensorDDGroup'
            , enableDragDrop: true
            , isTarget: true
            , tbar: { xtype: 'myproject-view-maptoolbar' }
            , layers: this.createLayers() // this only works because of the overridden layers member variable above
        });

        me.callParent(arguments);

        var map = me.map;

        map.events.register("mousemove", map, function(e) { 
            // do something
        });

    },

    createLayers: function() {        
        // ...
    }
});

The parent class has a member "layers" that is initially set to null. ApplyIf (as being the preferred way of merging config params by Sencha) will not work unless i manually override the layers member variable with undefined. Possible effects are, when using applyIf, the config param "layers" would not be added to the class.

[bartvde]

but I also see null values in Sencha's own codebase, e.g. value in Ext.picker.Color

Do you have a reference that states that applyIf is the preferred way by Sencha also for Ext 4?

[bartvde]

Also can you explain to me what the issue would be with using Ext.apply in your class? What could be unwanted side effects?

[hutzelknecht]

@bartvde Interesting that you found a null param in an extJS-class. There seems to be a different policy with cfg-params vs. private class members. The reference to the use of applyIf is coming from sencha architects default way of creating classes see this link for an example. Possible issues with using undefined vs. null would be whenever the GeoExt framework relies on typed comparison (===) to check if the variable is empty.

[bartvde]

Okay I think you have persuaded me @hutzelknecht I would support a PR to implement this

@marcjansen what about you?

[marcjansen]

To be honest I still do not see any problem.

When you subclass something and then set new hardcoded values in initComponent, you can as well set them in the class definition or not?

What would be needed to fix the issue?

Switch from initializing with null to initializing with undefined? This would be a major change which would also quite possibly might break existing 2.0-applications, right? Or am I exaggerating?

I do not want to introduce s.th. that makes us need to call the next version 2.1.0 if it is not absolutely necessary.

[marcjansen]

The effect of first setting layers to undefined is, that you can use it inside applyIf, right? The exact same thing could be achieved with

// ...
initComponent: function(){
    // ... use applyIf for you usual overwrites
    this.layers =  this.createLayers();
    this.callParent();
}

[bartvde]

Btw is anybody actually using GeoExt with Sencha Architect? If so, that could make this issue more real?

[marcjansen]

We are using architect but only for scaffolding and click dummies.

[hutzelknecht]

This is nothing really urgent. For me and my collegues this is now a known issue where we will use a workaround. Could be that only experienced extjs developers know of the benefits of using applyIf. Therefore developers new to the framework don't run into this kind of issue. My suggestion is to stick to extjs class definition patterns in the future. As stated by @marcjansen in a future minor release.

[zerotacg]

you use applyIf to apply defaults without loosing the ability to have it configured

initComponent: function(){
    this.layers =  this.createLayers(); // can't be overridden with config values
    this.layers =  this.layers || this.createLayers(); // to get the ability to override it via configs
    Ext.applyIf(this, {
        layers: this.createLayers() // for multiple defaults that can't be done as properties
    }
    this.callParent();
}

the applyIf version has the advantage that you don't have to repeat the variable twice it is a minor issue though, just a pitfall of applyIf