Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react-grid): prevent Sizer's infinite loop #2662

Merged
merged 6 commits into from
Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/dx-react-chart/src/plugins/pane-layout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('PaneLayout', () => {
</PluginHost>
));

expect(tree.find('Sizer').props()).toEqual({
expect(tree.find('UpdatableSizer').props()).toEqual({
containerComponent: expect.any(Function),
onSizeChange: expect.any(Function),
children: expect.anything(),
Expand Down
10 changes: 7 additions & 3 deletions packages/dx-react-chart/src/plugins/pane-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@ import {
Template,
TemplateConnector,
TemplatePlaceholder,
Sizer,
} from '@devexpress/dx-react-core';
import { ClipPath } from '../templates/clip-path';

// Original *Sizer* cannot be used because it ignores (as it should do) *forceUpdate* request.
// *UpdatableSizer* implements *componentDidUpdate* and forces internal *Sizer* size calculation.
// It allows to run chart size recalculation by calling *forceUpdate* on chart instance.
import { UpdatableSizer } from '../utils/updatable-sizer';

const DIV_STYLE: React.CSSProperties = {
flex: 1, zIndex: 1, position: 'relative', width: '100%',
};
Expand Down Expand Up @@ -41,7 +45,7 @@ export class PaneLayout extends React.PureComponent {
{({ layouts }, { changeBBox }) => {
const { width, height } = layouts.pane;
return (
<Sizer
<UpdatableSizer
containerComponent={SizerContainer}
onSizeChange={size => changeBBox({ placeholder: 'pane', bBox: size })}
>
Expand All @@ -55,7 +59,7 @@ export class PaneLayout extends React.PureComponent {
<ClipPath id={this.clipPathId} width={width} height={height} />
<TemplatePlaceholder name="series" />
</svg>
</Sizer>
</UpdatableSizer>
);
}}
</TemplateConnector>
Expand Down
37 changes: 37 additions & 0 deletions packages/dx-react-chart/src/utils/updatable-sizer.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as React from 'react';
import { mount } from 'enzyme';
import { UpdatableSizer } from './updatable-sizer';

describe('UpdatableSizer', () => {
it('should render original Sizer', () => {
const containerComponent = 'div';
const onSizeChange = () => 0;
const tree = mount(
<UpdatableSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
/>,
);

expect(tree.find('Sizer').props()).toEqual({
onSizeChange,
containerComponent,
});
});

it('should recalculate size on update', () => {
const containerComponent = 'div';
const onSizeChange = jest.fn();
const tree = mount(
<UpdatableSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
/>,
);
onSizeChange.mockClear();

tree.setProps({ test: 'tag' }); // force update

expect(onSizeChange).toBeCalled();
});
});
16 changes: 16 additions & 0 deletions packages/dx-react-chart/src/utils/updatable-sizer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as React from 'react';
import { Sizer, SizerProps } from '@devexpress/dx-react-core';

// It is located in a separate file only for testing purpose -
// it should actually be placed next to PaneLayout.
export class UpdatableSizer extends React.PureComponent<SizerProps> {
ref = React.createRef<Sizer>();

componentDidUpdate() {
this.ref.current!.setupListeners();
}

render() {
return <Sizer ref={this.ref} {...this.props} />;
}
}
16 changes: 0 additions & 16 deletions packages/dx-react-core/src/sizer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,4 @@ describe('Sizer', () => {
expect(expandNotifier.style.width).toBe('2px');
expect(expandNotifier.style.height).toBe('2px');
});

it('should recalculate size on update', () => {
const containerComponent = 'div';
const onSizeChange = jest.fn();
const tree = mount(
<Sizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
/>,
);
onSizeChange.mockClear();

tree.setProps({ test: 'tag' }); // force update

expect(onSizeChange).toBeCalled();
});
});
1 change: 0 additions & 1 deletion packages/dx-react-core/src/sizer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export class Sizer extends React.PureComponent<SizerProps> {
}

componentDidUpdate() {
this.setupListeners();
// We can scroll the VirtualTable manually only by changing
// containter's (rootNode) scrollTop property.
// Viewport changes its own properties automatically.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { findDOMNode } from 'react-dom';
import { shallow, mount } from 'enzyme';
import { Sizer } from '@devexpress/dx-react-core';
import { GridSizer } from '../../utils/grid-sizer';
import {
getCollapsedGrids,
TABLE_FLEX_TYPE,
Expand Down Expand Up @@ -55,6 +56,21 @@ jest.mock('@devexpress/dx-react-core', () => {
},
};
});
jest.mock('../../utils/grid-sizer', () => {
const { Component } = require.requireActual('react');
return {
...require.requireActual('../../utils/grid-sizer'),
// tslint:disable-next-line: max-classes-per-file
GridSizer: class extends Component {
render() {
const { collapsedGrid, ...restProps } = this.props;
return (
<Sizer {...restProps} />
);
}
},
};
});

// tslint:disable-next-line: max-classes-per-file
class VirtualTableLayoutWrapper extends React.Component<any, any> {
Expand Down Expand Up @@ -166,7 +182,7 @@ describe('VirtualTableLayout', () => {
/>
));

expect(tree.find(Sizer).dive())
expect(tree.find(GridSizer).dive().dive())
.toMatchSnapshot();
});

Expand All @@ -181,7 +197,7 @@ describe('VirtualTableLayout', () => {
/>
));

expect(tree.find(Sizer).prop('scrollTop'))
expect(tree.find(GridSizer).prop('scrollTop'))
.toEqual(scrollTop);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import { Sizer } from '@devexpress/dx-react-core';
import { MemoizedFunction, memoize } from '@devexpress/dx-core';
import {
TableColumn, GetColumnWidthFn, getCollapsedGrids,
Expand All @@ -9,6 +8,9 @@ import { VirtualTableLayoutState, VirtualTableLayoutProps } from '../../types';
import { findDOMNode } from 'react-dom';
import { VirtualTableLayoutBlock } from './virtual-table-layout-block';

// NOTE: Original Sizer doesn't work correctly with Material-UI Tabs (#2550)
import { GridSizer } from '../../utils/grid-sizer';

const AUTO_HEIGHT = 'auto';

const defaultProps = {
Expand Down Expand Up @@ -315,14 +317,15 @@ export class VirtualTableLayout extends React.PureComponent<PropsType, VirtualTa
};

return (
<Sizer
<GridSizer
onSizeChange={this.handleContainerSizeChange}
containerComponent={Container}
style={{
...(height === AUTO_HEIGHT ? null : { height }),
}}
onScroll={this.onScroll}
scrollTop={scrollTop}
collapsedGrid={collapsedGrids.headerGrid}
>
{
(!!headerRows.length) && (
Expand Down Expand Up @@ -355,7 +358,7 @@ export class VirtualTableLayout extends React.PureComponent<PropsType, VirtualTa
/>
)
}
</Sizer>
</GridSizer>
);
}
}
98 changes: 98 additions & 0 deletions packages/dx-react-grid/src/utils/grid-sizer.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import * as React from 'react';
import { mount } from 'enzyme';
import { GridSizer } from './grid-sizer';
import { TABLE_STUB_TYPE } from '@devexpress/dx-grid-core';

describe('GridSizer', () => {
it('should render original Sizer', () => {
const containerComponent = 'div';
const onSizeChange = () => 0;
const tree = mount(
<GridSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
/>,
);

expect(tree.find('Sizer').props()).toEqual({
onSizeChange,
containerComponent: 'div',
AryamnovEugeniy marked this conversation as resolved.
Show resolved Hide resolved
});
});

it('should call "onSizeChange" after update if columns include the stub', () => {
const containerComponent = 'div';
const onSizeChange = jest.fn();
const tree = mount(
<GridSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
collapsedGrid={{
columns: [{ type: 'a' }],
rows: [{ type: 'a' }],
}}
/>,
);
onSizeChange.mockClear();

tree.setProps({
collapsedGrid: {
columns: [{ type: TABLE_STUB_TYPE }],
rows: [{ type: 'a' }],
},
});

expect(onSizeChange).toBeCalled();
});

it('should call "onSizeChange" after update if rows include the stub', () => {
const containerComponent = 'div';
const onSizeChange = jest.fn();
const tree = mount(
<GridSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
collapsedGrid={{
columns: [{ type: 'a' }],
rows: [{ type: 'a' }],
}}
/>,
);
onSizeChange.mockClear();

tree.setProps({
collapsedGrid: {
columns: [{ type: 'a' }],
rows: [{ type: TABLE_STUB_TYPE }],
},
});

expect(onSizeChange).toBeCalled();
});

// tslint:disable-next-line: max-line-length
it('should not call "onSizeChange" after update if neither columns nor rows include the stub', () => {
const containerComponent = 'div';
const onSizeChange = jest.fn();
const tree = mount(
<GridSizer
containerComponent={containerComponent}
onSizeChange={onSizeChange}
collapsedGrid={{
columns: [{ type: 'a' }],
rows: [{ type: 'a' }],
}}
/>,
);
onSizeChange.mockClear();

tree.setProps({
collapsedGrid: {
columns: [{ type: 'b' }],
rows: [{ type: 'c' }],
},
});

expect(onSizeChange).not.toBeCalled();
});
});
25 changes: 25 additions & 0 deletions packages/dx-react-grid/src/utils/grid-sizer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as React from 'react';
import { Sizer, SizerProps } from '@devexpress/dx-react-core';
import { TABLE_STUB_TYPE, CollapsedGrid } from '@devexpress/dx-grid-core';
import { ReadonlyObject } from '@devexpress/dx-core';

// tslint:disable-next-line: max-line-length
export class GridSizer extends React.PureComponent<SizerProps & {collapsedGrid: ReadonlyObject<CollapsedGrid>}> {
ref = React.createRef<Sizer>();

componentDidUpdate() {
const { collapsedGrid: { rows, columns } } = this.props;

const hasStubColumn = columns.some(col => col.type === TABLE_STUB_TYPE);
const hasStubRow = rows.some(row => row.type === TABLE_STUB_TYPE);

if (hasStubColumn || hasStubRow) {
this.ref.current!.setupListeners();
}
}

render() {
const { collapsedGrid, ...restProps } = this.props;
return <Sizer ref={this.ref} {...restProps} />;
}
}