fix: specs swaps correctly reflected in state

src/state/chart_state.ts

@@ -336,7 +336,7 @@ export const chartStoreReducer = (chartId: string) => {
  ...state,
  specsInitialized: false,
  chartRendered: false,
- specParsing: true,
+ specParsing: false,
  specs: {
  ...rest,
  },

src/state/spec_factory.test.tsx

@@ -0,0 +1,94 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { mount, ReactWrapper } from 'enzyme';
+import React from 'react';
+
+import { BarSeries } from '../chart_types/xy_chart/specs/bar_series';
+import { Chart } from '../components/chart';
+import { MockSeriesSpec } from '../mocks/specs/specs';
+import { Settings } from '../specs/settings';
+import { DebugState } from './types';
+
+function getDebugState(wrapper: ReactWrapper): DebugState {
+ const statusComponent = wrapper.find('.echChartStatus');
+ const debugState = statusComponent.getDOMNode().getAttribute('data-ech-debug-state');
+ const parsedDebugState = JSON.parse(debugState || '');
+ return parsedDebugState as DebugState;
+}
+
+describe('Spec factory', () => {
+ const spec1 = MockSeriesSpec.bar({ id: 'spec1', data: [{ x: 0, y: 1 }] });
+ const spec2 = MockSeriesSpec.bar({ id: 'spec2', data: [{ x: 0, y: 2 }] });
+
+ it('We can switch specs props between react component', () => {
+ const wrapper = mount(
+ <Chart size={[100, 100]} id="chart1">
+ <Settings debugState />
+ <BarSeries {...spec1} />;
+ <BarSeries {...spec2} />;
+ </Chart>,
+ );
+ let debugState = getDebugState(wrapper);
+ expect(debugState.bars).toHaveLength(2);
+ wrapper.setProps({
+ children: (
+ <>
+ <Settings debugState />
+ <BarSeries {...spec2} />;
+ <BarSeries {...spec1} />;
+ </>
+ ),
+ });
+ debugState = getDebugState(wrapper);
+ expect(debugState.bars).toHaveLength(2);
+ });
+
+ it('We can switch specs ids between react component', () => {
+ const wrapper = mount(
+ <Chart size={[100, 100]} id="chart1">
+ <Settings debugState />
+ <BarSeries {...spec1} />;
+ <BarSeries {...spec2} />;
+ </Chart>,
+ );
+ let debugState = getDebugState(wrapper);
+ expect(debugState.bars).toHaveLength(2);
+ wrapper.setProps({
+ children: (
+ <>
+ <Settings debugState />
+ <BarSeries {...spec1} id={spec2.id} />;
+ <BarSeries {...spec2} id={spec1.id} />;
+ </>
+ ),
+ });
+ debugState = getDebugState(wrapper);
+
+ expect(debugState.bars).toHaveLength(2);
+
+ // different id same y
+ expect(debugState.bars?.[0].name).toBe('spec2');
+ expect(debugState.bars?.[0].bars[0].y).toBe(1);
+
+ // different id same y
+ expect(debugState.bars?.[1].name).toBe('spec1');
+ expect(debugState.bars?.[1].bars[0].y).toBe(2);
+ });
+});

src/state/spec_factory.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { useEffect, useRef } from 'react';
+import { useEffect } from 'react';
 import { connect } from 'react-redux';
 import { Dispatch, bindActionCreators } from 'redux';
 
@@ -30,26 +30,14 @@ export interface DispatchProps {
  removeSpec: (id: string) => void;
 }
 
-function usePrevious(value: string) {
- const ref = useRef<string>();
- useEffect(() => {
- ref.current = value;
- });
- return ref.current;
-}
-
 /** @internal */
 export function specComponentFactory<U extends Spec, D extends keyof U>(
  defaultProps: Pick<U, D | 'chartType' | 'specType'>,
 ) {
  /* eslint-disable no-shadow, react-hooks/exhaustive-deps, unicorn/consistent-function-scoping */
  const SpecInstance = (props: U & DispatchProps) => {
- const prevId = usePrevious(props.id);
  const { removeSpec, upsertSpec, ...SpecInstance } = props;
  useEffect(() => {
- if (prevId && prevId !== props.id) {
- removeSpec(prevId);
- }
  upsertSpec(SpecInstance);
  });
  useEffect(
@@ -80,7 +68,7 @@ export function getConnect() {
  * Redux assumes shallowEqual for all connected components
  *
  * This causes an issue where the specs are cleared and memoized spec components will never be
- * rerendered and thus never re-upserted to the state. Setting pure to false solves this issue
+ * re-rendered and thus never re-upserted to the state. Setting pure to false solves this issue
  * and doesn't cause traditional performance degradations.
  */
  return connect(null, mapDispatchToProps, null, { pure: false });