Results 1 to 10 of 10

Thread: Ext.data.Store - datachanged event fired on update

    Success! Looks like we've fixed this one. According to our records the fix was applied for EXTJS-25432 in 6.5.1.
  1. #1
    Sencha Premium User
    Join Date
    Sep 2008
    Posts
    140

    Default Ext.data.Store - datachanged event fired on update

    Ext version tested:


    • Ext 6.5.5


    Browser versions tested against:


    • Chrome


    Description:

    IMO this is a very serious bug as it causes several side-effects. The documentation is inconsistent as well. If a record in a store is updated the datachanged event is fired. It just doesn't make sense.

    From the documentation:
    datachanged - Fires whenever records are added to or removed from the Store.
    See: http://docs.sencha.com/extjs/6.5.0/c...nt-datachanged


    From the sources:
    src/data/Store.js
    PHP Code:
        onCollectionItemChange: function(collectioninfo) {
            var 
    me this,
                
    record info.item,
                
    modifiedFieldNames info.modified || null,
                
    type info.meta;


            if (
    me.fireChangeEvent(record)) {
                
    // Inform any interested parties that a record has been mutated.
                // This will be invoked on TreeStores in which the invoking record
                // is an descendant of a collapsed node, and so *will not be contained by this store
                
    me.onUpdate(recordtypemodifiedFieldNamesinfo);
                
    me.fireEvent('update'merecordtypemodifiedFieldNamesinfo);
                
    me.fireEvent('datachanged'me);
            }
        }, 
    In the sources there are multiple cases when update and datachanged are fired together.
    PHP Code:
                me.fireEvent('update'merecordExt.data.Model.COMMITmodifiedFieldNames);            me.fireEvent('datachanged'me); 

    Imo, update event is a record update event and datachanged should be fired ONLY if a record is being added/removed.
    Management means doing the things right,
    Leadership means doing the right things.
    www.interpid.eu

  2. #2
    Sencha Premium User evant's Avatar
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,245

    Default

    The intent of datachanged is to be a catch-all event for the store, for any kind of data change. It's largely useless outside of of that purpose, because you can't infer any context.

    We'll need to correct the documentation/upgrade guide to reflect that.
    Twitter - @evantrimboli
    Former Sencha framework engineer, available for consulting.
    As of 2017-09-22 I am not employed by Sencha, all subsequent posts are my own and do not represent Sencha in any way.

  3. #3
    Sencha Premium User
    Join Date
    Sep 2008
    Posts
    140

    Default

    Quote Originally Posted by evant View Post
    The intent of datachanged is to be a catch-all event for the store, for any kind of data change. It's largely useless outside of of that purpose, because you can't infer any context.

    We'll need to correct the documentation/upgrade guide to reflect that.
    Thanks for the answer. Is there any event that simulates the old behavior of datachanged?: "Fires whenever records are added to or removed from the Store?" Or, I'll have to work with the add/remove events.
    Management means doing the things right,
    Leadership means doing the right things.
    www.interpid.eu

  4. #4
    Sencha Premium User evant's Avatar
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,245

    Default

    For a long time, that has never really been the case. datachanged has been fired in pairs with every event (filter, sort, refresh, add, remove, write).

    If you're looking for a specific event you should listen to those directly.
    Twitter - @evantrimboli
    Former Sencha framework engineer, available for consulting.
    As of 2017-09-22 I am not employed by Sencha, all subsequent posts are my own and do not represent Sencha in any way.

  5. #5
    Sencha Premium User
    Join Date
    Apr 2015
    Location
    Germany
    Posts
    122

    Default

    Quote Originally Posted by evant View Post
    For a long time, that has never really been the case. datachanged has been fired in pairs with every event (filter, sort, refresh, add, remove, write).

    If you're looking for a specific event you should listen to those directly.
    In ExtJS 5.1.4 it has been the case. I used it in a store where data was loaded manually into with loadData method. Now I can't use the datachanged event there anymore because it fires too often. The refresh event is fired in that situation, too, but again, way too often. Load event is not fired in this situation, same for add. That means I can't use an event listener here anymore. That sucks. I thought at least the event definition should be stable when upgrading to a new ExtJS version.

    I don't get why this behaviour was changed. If you wanted to catch-all events before, you just had to listen to datachanged and update. That separation of concern was quite useful, at least for me.

  6. #6
    Sencha Premium User evant's Avatar
    Join Date
    Apr 2007
    Location
    Sydney, Australia
    Posts
    19,245

    Default

    If you wanted to catch-all events before, you just had to listen to datachanged and update.
    This is exactly the issue. If you wanted to catch everything, you had to listen to catch-all + something else. This was just bringing update inline with the others.
    Twitter - @evantrimboli
    Former Sencha framework engineer, available for consulting.
    As of 2017-09-22 I am not employed by Sencha, all subsequent posts are my own and do not represent Sencha in any way.

  7. #7
    Sencha Premium User
    Join Date
    Apr 2015
    Location
    Germany
    Posts
    122

    Default

    Quote Originally Posted by evant View Post
    This is exactly the issue. If you wanted to catch everything, you had to listen to catch-all + something else. This was just bringing update inline with the others.
    datachanged wasn't catch-all. Was update changed, too? If not, it wasn't brought in any line, either. Creating a catch-all event is great, if you want to catch-all. But added/removed items have to be handled way differently than edited items most of the time, so separating those cases was a good decision. Especially considering that not all situations previously handled by datachanged, are covered by any specific event now.

    Anyway... back to the specific problem with the loadData/loadRecords method. If you decide to keep that behaviour, there should be an event handling this store manipulation then. Should the load event be triggered here maybe? Well, I'm sure you'll find a solution.

  8. #8
    Sencha Premium User
    Join Date
    Nov 2013
    Location
    Piacenza, Italy
    Posts
    219

    Default

    I think this is really a bad change: if you needed a catch-all event you should have added a new one, instead of changing the behavior of an existing event, or at least you should have provided an new event that had the original behavior of "datachanged".

    Anyway, you changed the event meaning, but you did not update your own code, causing unwanted redraws and slow-downs!!!

    Examples:

    1)
    Ext.grid.feature.GroupStore.bindStore():
    Code:
            me.storeListeners = store.on({
                datachanged: me.onDataChanged,
                groupchange: me.onGroupChange,
                idchanged: me.onIdChanged,
                update: me.onUpdate,
                scope: me,
                destroyable: true
            });
    In case of "update" event, method onDataChanged() is called twice (because onUpdate() calls it).
    This, in turn, causes a "refresh" event to the grid everytime a record is updated!

    2)
    Ext.grid.feature.Summary.bindStore():
    Code:
            me.storeListeners = store.on({
                scope: me,
                destroyable: true,
                update: me.onStoreUpdate,
                datachanged: me.onStoreUpdate
            });
    In case of "update" event, method onStoreUpdate() is called twice (causing double work when a record is updated).

    3)
    Ext.ux.DataViewTransition.init():
    Code:
            dataview.store.on('datachanged', function(store) {
                /* ... */
            }, this);
    It seems to me this should listen only to "add" and "remove" events (instead now it forced to listen also to "update" events, causing unneeded work when a record is updated).

    4)
    Ext.ux.DataView.Animated.init():
    Code:
            dataview.store.on({
                datachanged: reDraw,
                scope: this,
                buffer: 50
            });
    It seems to me this should listen only to "add" and "remove" events (instead now it forced to listen also to "update" events, causing unneeded work when a record is updated).

  9. #9
    Sencha Premium User
    Join Date
    Nov 2013
    Location
    Piacenza, Italy
    Posts
    219

    Default

    I'm trying to fix myself Ext.grid.feature.GroupStore.bindStore(), but I cannot find a way to reproduce the old behavior with existing events.

    Look at these methods:
    - Ext.data.Store.onCollectionAddItems()
    - Ext.data.Store.onCollectionRemove()
    - Ext.data.ChainedStore.onCollectionAdd()
    - Ext.data.ChainedStore.onCollectionRemove()


    They all have a logic for buk "insert" or "remove" operations, like this:
    Code:
        onCollectionRemove: function(collection, info) {
            var me = this,
                records = info.items,
                lastChunk = !info.next;
            
            if (me.ignoreCollectionRemove) {
                return;
            }
            
            me.fireEvent('remove', me, records, info.at, false);
            // If there is a next property, that means there is another range that needs
            // to be removed after this. Wait until everything is gone before firign datachanged
            // since it should be a bulk operation
            if (lastChunk) {
                me.fireEvent('datachanged', me);
            }
        }

    If I listen to "datachanged" event I get also "update" events (which are not wanted), but if I listen to "add" and "remove" events, I loose the "bulk" operation optimization.

    So I still think that this change is a bug, or at least the change is not complete.

  10. #10
    Sencha Premium User
    Join Date
    Oct 2013
    Location
    Kharkiv, Ukraine
    Posts
    28

    Default

    BUMP: still happen in 6.6.0

Similar Threads

  1. Ext.data.Store datachanged-event
    By rsqw in forum Ext: Discussion
    Replies: 15
    Last Post: 28 Jan 2014, 7:51 AM
  2. Replies: 4
    Last Post: 22 Nov 2012, 3:10 AM
  3. Replies: 1
    Last Post: 8 Jul 2011, 7:43 AM
  4. Replies: 3
    Last Post: 3 Sep 2010, 6:28 PM
  5. datachanged event fired twice?
    By mitchellsimoens in forum Sencha Touch 1.x: Discussion
    Replies: 2
    Last Post: 11 Aug 2010, 8:17 AM

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •