fix #2452, and enhance option merge logic.

src/echarts.js

@@ -686,6 +686,7 @@ define(function (require) {
                 model = componentType;
             }
 
+            // Consider: id same and type changed.
             var viewId = model.id + '_' + model.type;
             var view = viewMap[viewId];
             if (!view) {

src/model/Global.js

@@ -131,79 +131,67 @@ define(function (require) {
             function visitComponent(mainType, dependencies) {
                 var newCptOptionList = newOption[mainType];
 
-                newCptOptionList
-                    ? handleNew.call(this, mainType, newCptOptionList, dependencies)
-                    : handleNoNew.call(this, mainType);
-
-                // Backup series for filtering.
-                if (mainType === 'series') {
-                    this._seriesIndices = createSeriesIndices(componentsMap.series);
-                }
-            }
-
-            function handleNoNew(mainType) {
-                // Possible when using removeEdgeAndAdd in topologicalTravel
-                // and ComponentModel.getAllClassMainTypes
-                each(componentsMap[mainType], function (cpt) {
-                    cpt.mergeOption({}, this);
-                }, this);
-            }
-
-            function handleNew(mainType, newCptOptionList, dependencies) {
-                // Normalize
                 if (!(zrUtil.isArray(newCptOptionList))) {
-                    newCptOptionList = [newCptOptionList];
-                }
-                if (!componentsMap[mainType]) {
-                    componentsMap[mainType] = [];
+                    newCptOptionList = newCptOptionList ? [newCptOptionList] : [];
                 }
 
-                var existComponents = mappingToExists(
-                    componentsMap[mainType], newCptOptionList
-                );
+                var mapResult = mappingToExists(componentsMap[mainType], newCptOptionList);
 
-                var keyInfoList = makeKeyInfo(
-                    mainType, newCptOptionList, existComponents
-                );
+                makeKeyInfo(mainType, mapResult);
 
                 var dependentModels = getComponentsByTypes(
                     componentsMap, dependencies
                 );
 
                 option[mainType] = [];
+                componentsMap[mainType] = [];
 
-                each(newCptOptionList, function (newCptOption, index) {
-                    if (!isObject(newCptOption)) {
-                        return;
-                    }
-
-                    var componentModel = existComponents[index];
+                each(mapResult, function (resultItem, index) {
+                    var componentModel = resultItem.exist;
+                    var newCptOption = resultItem.option;
 
-                    var ComponentModelClass = ComponentModel.getClass(
-                        mainType, keyInfoList[index].subType, true
+                    zrUtil.assert(
+                        isObject(newCptOption) || componentModel,
+                        'Empty component definition'
                     );
 
-                    if (componentModel && componentModel instanceof ComponentModelClass) {
-                        componentModel.mergeOption(newCptOption, this);
+                    // Consider where is no new option and should be merged using {},
+                    // see removeEdgeAndAdd in topologicalTravel and
+                    // ComponentModel.getAllClassMainTypes.
+                    if (!newCptOption) {
+                        componentModel.mergeOption({}, this);
                     }
                     else {
-                        // PENDING Global as parent ?
-                        componentModel = new ComponentModelClass(
-                            newCptOption, this, this,
-                            zrUtil.extend(
-                                {
-                                    dependentModels: dependentModels,
-                                    componentIndex: index
-                                },
-                                keyInfoList[index]
-                            )
+                        var ComponentModelClass = ComponentModel.getClass(
+                            mainType, resultItem.keyInfo.subType, true
                         );
-                        componentsMap[mainType][index] = componentModel;
+
+                        if (componentModel && componentModel instanceof ComponentModelClass) {
+                            componentModel.mergeOption(newCptOption, this);
+                        }
+                        else {
+                            // PENDING Global as parent ?
+                            componentModel = new ComponentModelClass(
+                                newCptOption, this, this,
+                                zrUtil.extend(
+                                    {
+                                        dependentModels: dependentModels,
+                                        componentIndex: index
+                                    },
+                                    resultItem.keyInfo
+                                )
+                            );
+                        }
                     }
 
-                    // Keep option
+                    componentsMap[mainType][index] = componentModel;
                     option[mainType][index] = componentModel.option;
                 }, this);
+
+                // Backup series for filtering.
+                if (mainType === 'series') {
+                    this._seriesIndices = createSeriesIndices(componentsMap.series);
+                }
             }
         },
 
@@ -595,46 +583,69 @@ define(function (require) {
     /**
      * @inner
      */
-    function mappingToExists(existComponents, newComponentOptionList) {
-        existComponents = (existComponents || []).slice();
-        var result = [];
+    function mappingToExists(existCpts, newCptOptions) {
+        // Mapping by the order by original option (but not order of
+        // new option) in merge mode. Because we should ensure
+        // some specified index (like xAxisIndex) is consistent with
+        // original option, which is easy to understand, espatially in
+        // media query. And in most case, merge option is used to
+        // update partial option but not be expected to change order.
+        newCptOptions = (newCptOptions || []).slice();
+
+        var result = map(existCpts || [], function (cpt, index) {
+            return {exist: cpt};
+        });
 
-        // Mapping by id if specified.
-        each(newComponentOptionList, function (componentOption, index) {
-            if (!isObject(componentOption) || !componentOption.id) {
+        // Mapping by id or name if specified.
+        each(newCptOptions, function (cptOption, index) {
+            if (!isObject(cptOption)) {
                 return;
             }
-            for (var i = 0, len = existComponents.length; i < len; i++) {
-                if (existComponents[i].id === componentOption.id) {
-                    result[index] = existComponents.splice(i, 1)[0];
-                    return;
+            for (var i = 0; i < result.length; i++) {
+                var exist = result[i].exist;
+                if (!result[i].option // Consider name: two map to one.
+                    && (
+                        // id has highest priority.
+                        (cptOption.id != null && exist.id === cptOption.id + '')
+                        || (cptOption.name != null
+                            && !isIdInner(cptOption)
+                            && !isIdInner(exist)
+                            && exist.name === cptOption.name + ''
+                        )
+                    )
+                ) {
+                    result[i].option = cptOption;
+                    newCptOptions[index] = null;
+                    break;
                 }
             }
         });
 
-        // Mapping by name if specified.
-        each(newComponentOptionList, function (componentOption, index) {
-            if (!isObject(componentOption)
-                || !componentOption.name
-                || hasInnerId(componentOption)
-            ) {
+        // Otherwise mapping by index.
+        each(newCptOptions, function (cptOption, index) {
+            if (!isObject(cptOption)) {
                 return;
             }
-            for (var i = 0, len = existComponents.length; i < len; i++) {
-                if (existComponents[i].name === componentOption.name) {
-                    result[index] = existComponents.splice(i, 1)[0];
-                    return;
+
+            var i = 0;
+            for (; i < result.length; i++) {
+                var exist = result[i].exist;
+                if (!result[i].option
+                    && !isIdInner(exist)
+                    // Caution:
+                    // Do not overwrite id. But name can be overwritten,
+                    // because axis use name as 'show label text'.
+                    // 'exist' always has id and name and we dont
+                    // need to check it.
+                    && cptOption.id == null
+                ) {
+                    result[i].option = cptOption;
+                    break;
                 }
             }
-        });
 
-        // Otherwise mapping by index.
-        each(newComponentOptionList, function (componentOption, index) {
-            if (!result[index]
-                && existComponents[index]
-                && !hasInnerId(componentOption)
-            ) {
-                result[index] = existComponents[index];
+            if (i >= result.length) {
+                result.push({option: cptOption});
             }
         });
 
@@ -644,95 +655,81 @@ define(function (require) {
     /**
      * @inner
      */
-    function makeKeyInfo(mainType, newCptOptionList, existComponents) {
+    function makeKeyInfo(mainType, mapResult) {
         // We use this id to hash component models and view instances
         // in echarts. id can be specified by user, or auto generated.
 
-        // The id generation rule ensures when setOption are called in
-        // no-merge mode, new model is able to replace old model, and
-        // new view instance are able to mapped to old instance.
-        // So we generate id by name and type.
+        // The id generation rule ensures new view instance are able
+        // to mapped to old instance when setOption are called in
+        // no-merge mode. So we generate model id by name and plus
+        // type in view id.
 
         // name can be duplicated among components, which is convenient
         // to specify multi components (like series) by one name.
 
-        // raw option should not be modified. for example, xxx.name might
-        // be rendered, so default name ('') should not be replaced by
-        // generated name. So we use keyInfoList wrap key info.
-        var keyInfoList = [];
+        // Ensure that each id is distinct.
+        var idMap = {};
 
-        // We use a prefix when generating name or id to prevent
-        // user using the generated name or id directly.
-        var prefix = '\0';
+        each(mapResult, function (item, index) {
+            var existCpt = item.exist;
+            var opt = item.option;
 
-        // Ensure that each id is distinct.
-        var idSet = {};
+            zrUtil.assert(
+                !opt || opt.id == null || !idMap[opt.id],
+                'id duplicates: ' + (opt && opt.id)
+            );
 
-        // key: name, value: count by single name.
-        var nameCount = {};
+            existCpt && (idMap[existCpt.id] = item);
+            opt && (idMap[opt.id] = item);
 
-        // Complete subType
-        each(newCptOptionList, function (opt, index) {
-            if (!isObject(opt)) {
-                return;
+            // Complete subType
+            if (isObject(opt)) {
+                var subType = determineSubType(mainType, opt, existCpt);
+                item.keyInfo = {mainType: mainType, subType: subType};
             }
-            var existCpt = existComponents[index];
-            var subType = determineSubType(mainType, opt, existCpt);
-            var item = {mainType: mainType, subType: subType};
-            keyInfoList[index] = item;
         });
 
-        function eachOpt(cb) {
-            each(newCptOptionList, function (opt, index) {
-                if (!isObject(opt)) {
-                    return;
-                }
-                var existCpt = existComponents[index];
-                var item = keyInfoList[index];
-                var fullType = mainType + '.' + item.subType;
-                cb(item, opt, existCpt, fullType);
-            });
-        }
+        // Make name and id.
+        each(mapResult, function (item, index) {
+            var existCpt = item.exist;
+            var opt = item.option;
+            var keyInfo = item.keyInfo;
+
+            if (!isObject(opt)) {
+                return;
+            }
 
-        // Make name
-        eachOpt(function (item, opt, existCpt, fullType) {
-            item.name = existCpt
+            // name can be overwitten. Consider case: axis.name = '20km'.
+            // But id generated by name will not be changed, which affect
+            // only in that case: setOption with 'not merge mode' and view
+            // instance will be recreated, which can be accepted.
+            keyInfo.name = opt.name != null
+                ? opt.name + ''
+                : existCpt
                 ? existCpt.name
-                : opt.name != null
-                ? opt.name
-                : prefix + '-';
-            // init nameCount
-            nameCount[item.name] = 0;
-        });
+                : '\0-';
 
-        // Make id
-        eachOpt(function (item, opt, existCpt, fullType) {
-            var itemName = item.name;
-
-            item.id = existCpt
-                ? existCpt.id
-                : opt.id != null
-                ? opt.id
-                // (1) Using delimiter to escapse dulipcation.
-                // (2) Using type tu ensure that view with different
-                //     type will not be mapped.
-                // (3) Consider this situatoin:
-                //      optionA: [{name: 'a'}, {name: 'a'}, {..}]
-                //      optionB [{..}, {name: 'a'}, {name: 'a'}]
-                //     Using nameCount to ensure that series with
-                //     the same name between optionA and optionB
-                //     can be mapped.
-                : prefix + [fullType, itemName, nameCount[itemName]++].join('|');
-
-            if (idSet[item.id]) {
-                // FIXME
-                // how to throw
-                throw new Error('id duplicates: ' + item.id);
+            if (existCpt) {
+                keyInfo.id = existCpt.id;
+            }
+            else if (opt.id != null) {
+                keyInfo.id = opt.id + '';
+            }
+            else {
+                // Consider this situatoin:
+                //  optionA: [{name: 'a'}, {name: 'a'}, {..}]
+                //  optionB [{..}, {name: 'a'}, {name: 'a'}]
+                // Series with the same name between optionA and optionB
+                // should be mapped.
+                var idNum = 0;
+                do {
+                    keyInfo.id = '\0' + keyInfo.name + '\0' + idNum++;
+                }
+                while (idMap[keyInfo.id]);
             }
-            idSet[item.id] = 1;
-        });
 
-        return keyInfoList;
+            idMap[keyInfo.id] = item;
+        });
     }
 
     /**
@@ -775,11 +772,9 @@ define(function (require) {
     /**
      * @inner
      */
-    function hasInnerId(componentOption) {
-        return componentOption.id
-            // FIXME
-            // Where to put this constant.
-            && (componentOption.id + '').indexOf('\0_ec_\0') === 0;
+    function isIdInner(cptOption) {
+        // FIXME: Where to put this constant.
+        return cptOption && cptOption.id && (cptOption.id + '').indexOf('\0_ec_\0') === 0;
     }
 
     /**

src/util/component.js

@@ -106,7 +106,7 @@ define(function(require) {
                 }
             }
 
-            // Consider this case: legend depends series, we call
+            // Consider this case: legend depends on series, and we call
             // chart.setOption({series: [...]}), where only series is in option.
             // If we do not have 'removeEdgeAndAdd', legendModel.mergeOption will
             // not be called, but only sereis.mergeOption is called. Thus legend

src/util/layout.js

@@ -289,11 +289,11 @@ define(function(require) {
      *
      * @param {Object} targetOption
      * @param {Object} newOption
-     * @param {Object} [opt]
+     * @param {Object|string} [opt]
      * @param {boolean} [opt.ignoreSize=false] Some component must has width and height.
      */
     layout.mergeLayoutParam = function (targetOption, newOption, opt) {
-        opt = opt || {};
+        !zrUtil.isObject(opt) && (opt = {});
         var hNames = ['width', 'left', 'right']; // Order by priority.
         var vNames = ['height', 'top', 'bottom']; // Order by priority.
         var hResult = merge(hNames);

test/ut/all.js

@@ -4,5 +4,6 @@ document.write('<script src="ut/core/utHelper.js"><\/script>');
 document.write('<script src="ut/spec/util/graphic.js"><\/script>');
 document.write('<script src="ut/spec/util/model.js"><\/script>');
 document.write('<script src="ut/spec/model/Component.js"><\/script>');
+document.write('<script src="ut/spec/model/Global.js"><\/script>');
 
 document.write('<script src="ut/spec/data/List.js"><\/script>');

test/ut/spec/.jshintrc

+{
+    "bitwise": false,
+    "camelcase": true,
+    "curly": true,
+    "es3": true,
+    "eqeqeq": true,
+    "forin": true,
+    "freeze": true,
+    "immed": true,
+    "latedef": "nofunc",
+    "newcap": true,
+    "noarg": true,
+    "noempty": true,
+    "nonbsp": true,
+    "nonew": true,
+    "plusplus": false,
+    "quotmark": "single",
+    "undef": true,
+    "predef": ["define", "require", "describe", "expect", "it"],
+    "unused": "vars",
+    "strict": false,
+    "maxparams": 20,
+    "maxdepth": 6,
+    "maxlen": 120,
+
+    "asi": false,
+    "boss": true,
+    "debug": false,
+    "eqnull": true,
+    "esnext": false,
+    "evil": false,
+    "expr": true,
+    "funcscope": false,
+    "globalstrict": false,
+    "iterator": false,
+    "lastsemic": false,
+    "laxbreak": true,
+    "laxcomma": false,
+    "loopfunc": false,
+    "multistr": false,
+    "notypeof": false,
+    "proto": false,
+    "scripturl": false,
+    "shadow": true,
+    "sub": true,
+    "supernew": false,
+    "validthis": true,
+
+    "browser": true,
+    "jasmine": true,
+    "couch": false,
+    "devel": true,
+    "dojo": false,
+    "jquery": false,
+    "mootools": false,
+    "node": true,
+    "nonstandard": true,
+    "prototypejs": false,
+    "rhino": false,
+    "wsh": true
+}