Hold sync during set_state?

[vidartf]

While reviewing #2256, I considered that maybe a more elegant solution would be to use hold_sync when setting the state from an update message. This would ensure that any new state changes as a direct response to the update (i.e. synchronous changes from validators or observers) all get collapsed into a single update message to the frontend. This collapsed update could then be verified against the property lock simplifying the logic.

A possible problem is that if multiple attributes change, then there is a potential for a change in the order of JS-side change notifications. For example:

• Previously a new state cause the updates: {foo: 5}, then {bar: 1}. This would trigger the change:foo and change events on the JS side first, with only foo updated. Then change:bar and change with bar updated.
• Now, the new state cause the update: {foo: 5, bar: 1}. This triggers change:foo and change:bar in undefined order, then a single change event, all with both foo and bar updated.

This could potentially be an issue for selections, e.g. here. In this case, the view-code would need to always update the options and index in the same go, using hasChanged to confirm e.g. like this:

initialize(parameters) {
    super.initialize(parameters);
    this.listenTo(this.model, 'change', (model, value, options) => this._updateSelection(options));
    // Create listbox here so that subclasses can modify it before it is populated in render()
    this.listbox = document.createElement('select');
}

updateSelection(options: any = {}) {
    // Debounce set calls from ourselves:
    if (options.updated_view === this) {
        return;
    }
    const optsChanged = this.hasChanged('_options_labels');
    const idxChanged = this.hasChanged('index');
    // The attributes we care about did not change, skip
    if (!idxChanged && !optsChanged) {
        return;
    }

    // Get the index first!
    const idx = this.model.get('index');

    if (optsChanged) {
        // Need to update options:
        ... // Code as before
    }
    // Always set the index
    this.listbox.selectedIndex = index === null ? -1 : index;
}

[maartenbreddels]

To fill in some things we considered during the meeting:
If not possible with backbone to handle these multiple changes in 1 event handler this may be an option to step away from it (cc @jasongrout)
A possible workaround would be to do the validation after a requestAnimationFrame, but that feels like a hack.
In any case, this should be a version 8.0 change.

[vidartf]

A possible workaround would be to do the validation after a requestAnimationFrame, but that feels like a hack.

I did not include this in the issue as I forgot that backbone actually sets all the attributes before any events are triggered. As such, this workaround should not do anything.

[vidartf]

Also: The fix for the selection widgets should be used anyways, as all widgets should deal with set being called with multiple arguments at the same time. As it stands now, the selection controls are likely to fail if hold_sync is used, which should be fixed!