This is an automated email from the ASF dual-hosted git repository. sushuang pushed a commit to branch remove-component in repository https://gitbox.apache.org/repos/asf/incubator-echarts.git
commit 1db1349b17adfe7924483bab289f78873a4855e0 Author: 100pah <sushuang0...@gmail.com> AuthorDate: Tue Jul 14 17:28:21 2020 +0800 feature: In replaceMerge, trade {xxx: null/undefined} the same as {xxx: []} for ec option. --- src/model/Global.ts | 60 +++++++++++++++++++--------- test/option-replaceMerge.html | 93 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 18 deletions(-) diff --git a/src/model/Global.ts b/src/model/Global.ts index 5f5555e..ad02e72 100644 --- a/src/model/Global.ts +++ b/src/model/Global.ts @@ -62,7 +62,7 @@ export interface GlobalModelSetOptionOpts { replaceMerge: ComponentMainType | ComponentMainType[]; } export interface InnerSetOptionOpts { - replaceMergeMainTypeMap: HashMap<boolean>; + replaceMergeMainTypeMap: HashMap<boolean, string>; } // ----------------------- @@ -81,7 +81,7 @@ class GlobalModel extends Model<ECUnitOption> { private _optionManager: OptionManager; - private _componentsMap: HashMap<ComponentModel[]>; + private _componentsMap: HashMap<ComponentModel[], string>; /** * `_componentsMap` might have "hole" becuase of remove. @@ -218,6 +218,7 @@ class GlobalModel extends Model<ECUnitOption> { const componentsMap = this._componentsMap; const componentsCount = this._componentsCount; const newCmptTypes: ComponentMainType[] = []; + const newCmptTypeMap = createHashMap<boolean, string>(); const replaceMergeMainTypeMap = opt && opt.replaceMergeMainTypeMap; resetSourceDefaulter(this); @@ -237,9 +238,23 @@ class GlobalModel extends Model<ECUnitOption> { } else if (mainType) { newCmptTypes.push(mainType); + newCmptTypeMap.set(mainType, true); } }); + if (replaceMergeMainTypeMap) { + // If there is a mainType `xxx` in `replaceMerge` but not declared in option, + // we trade it as it is declared in option as `{xxx: []}`. Because: + // (1) for normal merge, `{xxx: null/undefined}` are the same meaning as `{xxx: []}`. + // (2) some preprocessor may convert some of `{xxx: null/undefined}` to `{xxx: []}`. + replaceMergeMainTypeMap.each(function (b, mainTypeInReplaceMerge) { + if (!newCmptTypeMap.get(mainTypeInReplaceMerge)) { + newCmptTypes.push(mainTypeInReplaceMerge); + newCmptTypeMap.set(mainTypeInReplaceMerge, true); + } + }); + } + (ComponentModel as ComponentModelConstructor).topologicalTravel( newCmptTypes, (ComponentModel as ComponentModelConstructor).getAllClassMainTypes(), @@ -249,10 +264,8 @@ class GlobalModel extends Model<ECUnitOption> { function visitComponent( this: GlobalModel, - mainType: ComponentMainType, - dependencies: string | string[] + mainType: ComponentMainType ): void { - const newCmptOptionList = modelUtil.normalizeToArray(newOption[mainType]); const oldCmptList = componentsMap.get(mainType); @@ -263,11 +276,13 @@ class GlobalModel extends Model<ECUnitOption> { // Set mainType and complete subType. modelUtil.setComponentTypeToKeyInfo(mappingResult, mainType, ComponentModel as ComponentModelConstructor); - // Set it before the travel, in case that `this._componentsMap` is - // used in some `init` or `merge` of components. + // Empty it before the travel, in order to prevent `this._componentsMap` + // from being used in the `init`/`mergeOption`/`optionUpdated` of some + // components, which is probably incorrect logic. option[mainType] = null; componentsMap.set(mainType, null); componentsCount.set(mainType, 0); + const optionsByMainType = [] as ComponentOption[]; const cmptsByMainType = [] as ComponentModel[]; let cmptsCountByMainType = 0; @@ -794,8 +809,8 @@ export interface QueryConditionKindB { mainType: ComponentMainType; subType?: ComponentSubType; index?: number | number[]; - id?: string | string[]; - name?: string | string[]; + id?: string | number | (string | number)[]; + name?: (string | number) | (string | number)[]; } export interface EachComponentAllCallback { (mainType: string, model: ComponentModel, componentIndex: number): void; @@ -844,16 +859,25 @@ function mergeTheme(option: ECUnitOption, theme: ThemeOption): void { function queryByIdOrName<T extends { id?: string, name?: string }>( attr: 'id' | 'name', - idOrName: string | string[], + idOrName: string | number | (string | number)[], cmpts: T[] ): T[] { - let keyMap: HashMap<string>; - return isArray(idOrName) - ? ( - keyMap = createHashMap(idOrName), - filter(cmpts, cmpt => cmpt && keyMap.get(cmpt[attr]) != null) - ) - : filter(cmpts, cmpt => cmpt && cmpt[attr] === idOrName + ''); + // Here is a break from echarts4: string and number-like string are + // traded as equal. + if (isArray(idOrName)) { + const keyMap = createHashMap<boolean>(idOrName); + each(idOrName, function (idOrNameItem) { + if (idOrNameItem != null) { + modelUtil.validateIdOrName(idOrNameItem); + keyMap.set(idOrNameItem, true); + } + }); + return filter(cmpts, cmpt => cmpt && keyMap.get(cmpt[attr])); + } + else { + modelUtil.validateIdOrName(idOrName); + return filter(cmpts, cmpt => cmpt && cmpt[attr] === idOrName + ''); + } } function filterBySubType( @@ -868,7 +892,7 @@ function filterBySubType( } function normalizeReplaceMergeInput(opts: GlobalModelSetOptionOpts): InnerSetOptionOpts { - const replaceMergeMainTypeMap = createHashMap<boolean>(); + const replaceMergeMainTypeMap = createHashMap<boolean, string>(); opts && each(modelUtil.normalizeToArray(opts.replaceMerge), function (mainType) { if (__DEV__) { assert( diff --git a/test/option-replaceMerge.html b/test/option-replaceMerge.html index 30dc848..e525e7a 100644 --- a/test/option-replaceMerge.html +++ b/test/option-replaceMerge.html @@ -48,6 +48,7 @@ under the License. <div id="main_replaceMerge_remove_all"></div> <div id="main_replaceMerge_reproduce_by_getOption_src"></div> <div id="main_replaceMerge_reproduce_by_getOption_tar"></div> + <div id="main_replaceMerge_if_not_declared_in_option"></div> @@ -911,6 +912,98 @@ under the License. + + + + + + <script> + require(['echarts'], function (echarts) { + var option; + + option = { + // toolbox: { + // left: 'center', + // feature: { + // dataZoom: {} + // } + // }, + grid: [{ + bottom: '60%' + }, { + id: 'gb', + top: '60%' + }], + xAxis: [{ + type: 'category' + }, { + type: 'category' + }, { + id: 'xb0', + type: 'category', + gridIndex: 1 + }, { + id: 'xb1', + type: 'category', + gridIndex: 1 + }], + yAxis: [{ + + }, { + id: 'yb', + gridIndex: 1 + }], + dataZoom: [{ + type: 'slider' + }, { + type: 'inside' + }], + series: [{ + type: 'line', + data: [[11, 22], [33, 44]] + }, { + type: 'line', + xAxisIndex: 1, + data: [[11111, 52], [21133, 74]] + }, { + id: 'sb0', + type: 'line', + xAxisIndex: 2, + yAxisIndex: 1, + data: [[23, 432], [54, 552]] + }, { + id: 'sb1', + type: 'line', + xAxisIndex: 3, + yAxisIndex: 1, + data: [[222233, 1432], [111154, 1552]] + }] + }; + + var chart = testHelper.create(echarts, 'main_replaceMerge_if_not_declared_in_option', { + title: [ + 'Click btn "remove all grids".', + 'Should no error.', + 'Click btn "addback the first grid".', + 'Functionalities should be OK.' + ], + option: option, + height: 350, + buttons: [{ + text: 'remove all grids', + onclick: function () { + chart.setOption({ + // Not declared in option + }, { replaceMerge: ['grid', 'xAxis', 'yAxis', 'series'] }); + } + }] + }); + }); + </script> + + + + </body> </html> --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org For additional commands, e-mail: commits-h...@echarts.apache.org