You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by su...@apache.org on 2020/07/17 11:39:08 UTC
[incubator-echarts] 09/16: feature: add test case for id duplicated.
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 a3d29cf60002869679d58259b9f9d2b2758bbcec
Author: 100pah <su...@gmail.com>
AuthorDate: Tue Jul 14 18:09:46 2020 +0800
feature: add test case for id duplicated.
---
src/model/Global.ts | 2 +-
src/util/model.ts | 32 ++++++++++++++----------
test/lib/testHelper.js | 19 ++++++++++----
test/option-replaceMerge.html | 58 +++++++++++++++++++++++++++++++++++++++++--
4 files changed, 90 insertions(+), 21 deletions(-)
diff --git a/src/model/Global.ts b/src/model/Global.ts
index ad02e72..5fd20d9 100644
--- a/src/model/Global.ts
+++ b/src/model/Global.ts
@@ -384,7 +384,7 @@ class GlobalModel extends Model<ECUnitOption> {
let metNonInner = false;
for (let i = realLen - 1; i >= 0; i--) {
// Remove options with inner id.
- if (opts[i] && !modelUtil.isIdInner(opts[i])) {
+ if (opts[i] && !modelUtil.isComponentIdInternal(opts[i])) {
metNonInner = true;
}
else {
diff --git a/src/util/model.ts b/src/util/model.ts
index 7e21a2e..e0939a9 100644
--- a/src/util/model.ts
+++ b/src/util/model.ts
@@ -56,6 +56,8 @@ import { isNumeric } from './number';
*/
const DUMMY_COMPONENT_NAME_PREFIX = 'series\0';
+const INTERNAL_COMPONENT_ID_PREFIX = '\0_ec_\0';
+
/**
* If value is not array, then translate it to array.
* @param {*} value
@@ -242,8 +244,8 @@ function mappingToExistsInNormalMerge<T extends MappingExistingItem>(
// Can not match when both ids existing but different.
&& existing
&& (existing.id == null || cmptOption.id == null)
- && !isIdInner(cmptOption)
- && !isIdInner(existing)
+ && !isComponentIdInternal(cmptOption)
+ && !isComponentIdInternal(existing)
&& keyExistAndEqual('name', existing, cmptOption)
) {
result[i].newOption = cmptOption;
@@ -264,7 +266,7 @@ function mappingToExistsInNormalMerge<T extends MappingExistingItem>(
* Mapping to exists for merge.
* The mode "replaceMerge" means that:
* (1) Only the id mapped components will be merged.
- * (2) Other existing components (except inner compoonets) will be removed.
+ * (2) Other existing components (except internal compoonets) will be removed.
* (3) Other new options will be used to create new component.
* (4) The index of the existing compoents will not be modified.
* That means their might be "hole" after the removal.
@@ -286,20 +288,20 @@ function mappingToExistsInReplaceMerge<T extends MappingExistingItem>(
// contains elided items, which will be ommited.
for (let index = 0; index < existings.length; index++) {
const existing = existings[index];
- let innerExisting: T;
+ let internalExisting: T;
// Because of replaceMerge, `existing` may be null/undefined.
if (existing) {
- if (isIdInner(existing)) {
- // inner components should not be removed.
- innerExisting = existing;
+ if (isComponentIdInternal(existing)) {
+ // internal components should not be removed.
+ internalExisting = existing;
}
- // Input with inner id is allowed for convenience of some internal usage.
+ // Input with internal id is allowed for convenience of some internal usage.
// When `existing` is rawOption (see `OptionManager`#`mergeOption`), id might be empty.
if (existing.id != null) {
existingIdIdxMap.set(existing.id, index);
}
}
- result.push({ existing: innerExisting });
+ result.push({ existing: internalExisting });
}
// Mapping by id if specified.
@@ -350,7 +352,7 @@ function mappingByIndexFinally<T extends MappingExistingItem>(
return;
}
- // Find the first place that not mapped by id and not inner component (consider the "hole").
+ // Find the first place that not mapped by id and not internal component (consider the "hole").
let resultItem;
while (
// Be `!resultItem` only when `nextIdx >= mappingResult.length`.
@@ -368,7 +370,7 @@ function mappingByIndexFinally<T extends MappingExistingItem>(
&& !keyExistAndEqual('id', cmptOption, resultItem.existing)
)
|| resultItem.newOption
- || isIdInner(resultItem.existing)
+ || isComponentIdInternal(resultItem.existing)
)
) {
nextIdx++;
@@ -414,6 +416,7 @@ function makeIdAndName(
each(mapResult, function (item) {
const opt = item.newOption;
+ // Force ensure id not duplicated.
assert(
!opt || opt.id == null || !idMap.get(opt.id) || idMap.get(opt.id) === item,
'id duplicates: ' + (opt && opt.id)
@@ -511,12 +514,15 @@ export function isNameSpecified(componentModel: ComponentModel): boolean {
* @param {Object} cmptOption
* @return {boolean}
*/
-export function isIdInner(cmptOption: MappingExistingItem): boolean {
+export function isComponentIdInternal(cmptOption: MappingExistingItem): boolean {
return cmptOption
&& cmptOption.id != null
- && makeComparableKey(cmptOption.id).indexOf('\0_ec_\0') === 0;
+ && makeComparableKey(cmptOption.id).indexOf(INTERNAL_COMPONENT_ID_PREFIX) === 0;
}
+export function makeInternalComponentId(idSuffix: string) {
+ return INTERNAL_COMPONENT_ID_PREFIX + idSuffix;
+}
export function setComponentTypeToKeyInfo(
mappingResult: MappingResult<MappingExistingItem & { subType?: ComponentSubType }>,
diff --git a/test/lib/testHelper.js b/test/lib/testHelper.js
index f5a5f0f..88dc879 100644
--- a/test/lib/testHelper.js
+++ b/test/lib/testHelper.js
@@ -275,10 +275,19 @@
* `testHelper.printAssert` can be called multiple times for one chart instance.
* For each call, one result (fail or pass) will be printed.
*
- * @param chart {EChartsInstance}
+ * @param chartOrDomId {EChartsInstance | string}
* @param checkFn {Function} param: a function `assert`.
*/
- testHelper.printAssert = function (chart, checkerFn) {
+ testHelper.printAssert = function (chartOrDomId, checkerFn) {
+ var hostDOMEl;
+ var chart;
+ if (typeof chartOrDomId === 'string') {
+ hostDOMEl = document.getElementById(chartOrDomId);
+ }
+ else {
+ chart = chartOrDomId;
+ hostDOMEl = chartOrDomId.getDom();
+ }
var failErr;
function assert(cond) {
if (!cond) {
@@ -292,7 +301,7 @@
console.error(err);
failErr = err;
}
- var printAssertRecord = chart.__printAssertRecord || (chart.__printAssertRecord = []);
+ var printAssertRecord = hostDOMEl.__printAssertRecord || (hostDOMEl.__printAssertRecord = []);
var resultDom = document.createElement('div');
resultDom.innerHTML = failErr ? 'checked: Fail' : 'checked: Pass';
@@ -305,12 +314,12 @@
'color: ' + (failErr ? 'red' : 'green') + ';',
].join('');
printAssertRecord.push(resultDom);
- chart.getDom().appendChild(resultDom);
+ hostDOMEl.appendChild(resultDom);
relayoutResult();
function relayoutResult() {
- var chartHeight = chart.getHeight();
+ var chartHeight = chart ? chart.getHeight() : hostDOMEl.offsetHeight;
var lineHeight = Math.min(fontSize + 10, (chartHeight - 20) / printAssertRecord.length);
for (var i = 0; i < printAssertRecord.length; i++) {
var record = printAssertRecord[i];
diff --git a/test/option-replaceMerge.html b/test/option-replaceMerge.html
index e525e7a..1c8ff5f 100644
--- a/test/option-replaceMerge.html
+++ b/test/option-replaceMerge.html
@@ -44,11 +44,12 @@ under the License.
<div id="main_replaceMerge_add_new_id"></div>
<div id="main_replaceMerge_add_find_hole"></div>
<div id="main_normalMerge_add_find_hole"></div>
- <div id="main_replaceMerge_inner_and_other_cmpt_not_effect"></div>
+ <div id="main_replaceMerge_internal_and_other_cmpt_not_effect"></div>
<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>
+ <div id="main_throw_error_when_id_duplicated"></div>
@@ -627,7 +628,7 @@ under the License.
}]
};
- var chart = testHelper.create(echarts, 'main_replaceMerge_inner_and_other_cmpt_not_effect', {
+ var chart = testHelper.create(echarts, 'main_replaceMerge_internal_and_other_cmpt_not_effect', {
title: [
'replaceMerge: inner not effect',
'click "setOption": a dataZoom.slider added',
@@ -1004,6 +1005,59 @@ under the License.
+
+
+
+
+
+ <script>
+ require(['echarts'], function (echarts) {
+ var option;
+
+ option = {
+ xAxis: [{
+ type: 'category'
+ }],
+ yAxis: [{
+ }],
+ series: [{
+ id: 'sb0',
+ type: 'line',
+ data: [[11, 22], [33, 44]]
+ }, {
+ type: 'line',
+ data: [[11111, 52], [21133, 74]]
+ }, {
+ id: 'sb0',
+ type: 'line',
+ data: [[222233, 1432], [111154, 1552]]
+ }]
+ };
+
+ testHelper.printAssert('main_throw_error_when_id_duplicated', function (assert) {
+ try {
+ var chart = testHelper.create(echarts, 'main_throw_error_when_id_duplicated', {
+ title: [
+ 'Check throw error when id duplicated',
+ 'Should show **checked: Pass**'
+ ],
+ option: option,
+ height: 100,
+ });
+ assert(false);
+ }
+ catch (err) {
+ assert(true);
+ }
+ });
+
+ });
+ </script>
+
+
+
+
+
</body>
</html>
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org