Skip to content

Commit

Permalink
fix: infinite loop when Line strokeDasharray is 0 (#3904)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
Fix infinite loop case in `Line` where `strokeDasharray="0"` causes an
infinite loop

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please link to the issue here: -->
#3899

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
This is a code smell and a bug

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
- Ran storybook with strokeDasharray as 0 and reproduced, made changes
and see that infinite loop no longer happens (and animation still works
as expected
- Add unit test

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [ ] I have added a storybook story or extended an existing story to
show my changes
- [x] All new and existing tests passed.

---------

Co-authored-by: Coltin Kifer <ckifer@amazon.com>
  • Loading branch information
ckifer and Coltin Kifer committed Oct 25, 2023
1 parent b2c099b commit e04f17a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
15 changes: 12 additions & 3 deletions src/cartesian/Line.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,24 @@ export class Line extends PureComponent<Props, State> {
}
}

generateSimpleStrokeDasharray = (totalLength: number, length: number): string => {
return `${length}px ${totalLength - length}px`;
};

getStrokeDasharray = (length: number, totalLength: number, lines: number[]) => {
const lineLength = lines.reduce((pre, next) => pre + next);

// if lineLength is 0 return the default when no strokeDasharray is provided
if (!lineLength) {
return this.generateSimpleStrokeDasharray(totalLength, length);
}

const count = Math.floor(length / lineLength);
const remainLength = length % lineLength;
const restLength = totalLength - length;

let remainLines = [];
for (let i = 0, sum = 0; ; sum += lines[i], ++i) {
let remainLines: number[] = [];
for (let i = 0, sum = 0; i < lines.length; sum += lines[i], ++i) {
if (sum + lines[i] > remainLength) {
remainLines = [...lines.slice(0, i), remainLength - sum];
break;
Expand Down Expand Up @@ -434,7 +443,7 @@ export class Line extends PureComponent<Props, State> {
const lines = `${strokeDasharray}`.split(/[,\s]+/gim).map(num => parseFloat(num));
currentStrokeDasharray = this.getStrokeDasharray(curLength, totalLength, lines);
} else {
currentStrokeDasharray = `${curLength}px ${totalLength - curLength}px`;
currentStrokeDasharray = this.generateSimpleStrokeDasharray(totalLength, curLength);
}

return this.renderCurveStatically(points, needClip, clipPathId, {
Expand Down
15 changes: 14 additions & 1 deletion test/cartesian/Line.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('<Line />', () => {
{ x: 170, y: 50, value: 100 },
];

it('Render a path in a simple Line', () => {
it('Renders a path in a simple Line', () => {
const { container } = render(
<Surface width={500} height={500}>
<Line isAnimationActive={false} points={data} />
Expand All @@ -21,6 +21,19 @@ describe('<Line />', () => {
expect(container.querySelectorAll('.recharts-line-curve')).toHaveLength(1);
});

it('Does not fall into infinite loop if strokeDasharray is 0', () => {
const { container } = render(
<Surface width={500} height={500}>
<Line points={data} strokeDasharray="0" />
</Surface>,
);

const line = container.querySelectorAll('.recharts-line-curve');
expect(line).toHaveLength(1);

expect(line[0].getAttribute('stroke-dasharray')).toEqual('0px 0px');
});

it('Does not throw when dot is null', () => {
const { container } = render(
<Surface width={500} height={500}>
Expand Down

0 comments on commit e04f17a

Please sign in to comment.