recharts / react-smooth

react animation
MIT License
277 stars 38 forks source link

Animation breaks because of NaN in easing #12

Closed Venryx closed 7 years ago

Venryx commented 7 years ago

The chart animation breaks sometimes in my ui, when unmounting and mounting frequently.

I narrowed it down to this function, in easing.js:

var multyTime = function multyTime(params, t) {
  return params.map(function (param, i) {
    return param * Math.pow(t, i);
  }).reduce(function (pre, curr) {
    return pre + curr;
  });

When this function is supplied with these arguments, it returns NaN:

This NaN value eventually pass all the way up to renderVerticalRect in Area.js, at which point an error occurs.

For now, I avoid the problem by just adding a check inside multyTime, which replaces NaN with 0. But it would be good to have some fix in the master repo.

EDIT

Interesting. So while the arguments above were indeed the exact values used when the NaN was produced, when running it again as a watch or in the console (while paused in the debugger), it does not return NaN.

However, when I run a loop, running that exact same line, it does produce NaN!

Not only that: the loop has to have a certain number of loops to trigger it!

I used binary search, and narrowed it down to a single number: 8942

In other words, when I run the following, "NaN produced!" is output to the console:

for (var i = 0; i < 8942; i++) {
    if (Number.isNaN(multyTime([0,0.30000000000000004,2.4,-1.7000000000000002], 0.9196919176875596)))
        console.log("NaN produced!");
}

But when I run this next version, "NaN produced!" is not output to the console:

for (var i = 0; i < 8941; i++) {
    if (Number.isNaN(multyTime([0,0.30000000000000004,2.4,-1.7000000000000002], 0.9196919176875596)))
        console.log("NaN produced!");
}

And this is completely repeatable. I did it back and forth many times, and it always triggers when the loop-count is 8942 or over, and never when it's under.

EDIT2

Okay, I did further testing by running the above, not while paused in the debugger, but just calling multyTime from the console. There, the issue above is not present. Instead, "NaN produced" is never produced, even when it's run with a loop-count of 10,000,000. So something weird was going on there.

EDIT3

Okay, I finally reproduced the problem to something that you can just paste into a console window (for example, on this page), and see the NaN result. Here it is:

// if you decrease the loop-count by even one, the `NaN` will not appear
// (possibly it's platform-dependent, so try a higher number if you don't get the NaN detection + log right away)
for (var i = 0; i < 6086; i++) { 
    var params = [0,0.75,-.75,1];
    var t = 0;
    var result1 = params.map(function(param, i) {
        return param * Math.pow(t, i);
    });
    var result2 = result1.reduce(function(pre, curr) {
        return pre + curr;
    });
    if (Number.isNaN(result2)) {
        console.log("NaN produced at loop: " + i);
        break;
    }
}

Once the NaN was produced, I then checked the value of result1 and result2.

result1: [0, 0, -0, 0] result2: NaN

Interesting.

However, when I re-ran the code that gave result2's NaN:

result1.reduce(function (pre, curr) {
    return pre + curr;
});

It returned 0.

So, it seems to be a strange and rare bug in Chrome, that only happens when you run a particular sequence of code many times in a row. But at least it's reproducible! So I should be able to submit a bug report, and hopefully they'll make a fix eventually.

Anyway, given that this bug does exist (you can try it right now to verify), and that it sometimes happens in actual usage of this library, should there perhaps be some sort of handling for it to prevent the resulting animation-breakage issue?

Technical info

OS Name: Microsoft Windows 10 Pro Version: 10.0.14393 Build 14393 System Type: x64-based PC Processor: AMD FX(tm)-8320 Eight-Core Processor, 3500 Mhz, 4 Core(s), 8 Logical Processor(s)

Chrome: "Version 59.0.3063.4 (Official Build) dev (64-bit)"

Note: I tried the code above (also with a higher loop-count), and the error was not reproduced in Chromium, Firefox, or Edge. Scratch that: the error is reproduced in Chromium, I just needed to update it to the latest version. (from "Version 59.0.3053.0 (Official Build) (64-bit)" to "Version 59.0.3060.0 (Official Build) (64-bit)")

Good, so now we even know what update range introduced the problem. Now time to make the Chrome/Chromium bug-report...

xile611 commented 7 years ago

@Venryx Would you want to submit a PR?

Venryx commented 7 years ago

Not to worry, I made a bug report, and they fixed it already. Newest versions of Chrome don't/won't have the issue.