undefined in params breaks animation - Regression between 3.1.1 and 3.2.0

[Wix Wow!Team]

Hi @GreenSock

I upgraded GSAP version in our project and some animations stopped working.

I managed to reduce it to afromTo that had an undefined value in the from params.

It used to work in 2.x and in 3.1.x but since 3.2.0 (and in the current 3.4.2) the animation fails.

 

One can say - "just don't have undefined fields in your code"

but as coding goes, it's not that simple and will require some serious refactoring on our part which I would prefer to avoid.

 

Working demo (GSAP 3.1.1) - 

 

Failing demo (GSAP 3.2.0) -

See the Pen VwegBKR by tombigel (@tombigel) on CodePen

See the Pen OJMdwRV by tombigel (@tombigel) on CodePen

[GreenSock]

It's a bit tricky because you're telling GSAP to set the skewX literally to undefined initially which is  not valid. When GSAP obeys your instruction, the browser refuses to apply the invalid transform value. Since ALL transforms (skewX, skewY, x, y, scale, rotation, etc.) get merged into a single "transform" property, it's tainted by that undefined value. 

 

For performance and file size reasons, we really don't want to inject a bunch of logic into GSAP to validate all possible CSS values. Like what if someone forgets the "x" in "px" or defines a "border" that doesn't have a border-style as a part of it, etc.? We try to make GSAP as performant and robust as possible, and there are intentional tradeoffs we make. In this case, checking for invalid values. Ultimately I think this is better for everyone and it encourages developers to write cleaner code. In v2 and earlier, I employed a bunch of strategies (and code) to try to "read the developer's mind" and elegantly recover when developers wrote code that really shouldn't work...but in retrospect I think that was not the best choice. When I re-engineered v3, I cleaned out a lot of that to streamline things and allow developers to bump into "problems" in their code so that they'd resolve things and ultimately write cleaner code. See what I mean? 

 

Even 3.1.1 doesn't really work in the scenario you described, as you can see here: 

See the Pen 755d26889f3f7858daf707604e008d28?editors=0010 by GreenSock (@GreenSock) on CodePen

 

It looks like you just stumbled upon an edge case that happened to work in an old version coincidentally with skews . But again, I wouldn't really expect it to work. I'd strongly recommend against telling GSAP to set values that are invalid.

 

That being said, I realize you've got a large codebase and it may be painful to clean things up on your side. I do have a way to at least skip a CSS value altogether if it's undefined. It wasn't very expensive. I've added that to the next release which you can preview at

https://assets.codepen.io/16327/gsap-latest-beta.min.js

 

Better? 

[GreenSock]

By the way, I forgot to thank you for providing such a nice reduced test case and identifying the exact version number where you see the regression. 🙌

 

Super helpful. 

[Wix Wow!Team]

Thanks a lot Jack for the quick reply

 

If you are saying that the previous behaviour is limited to a limited group of cases, that it is less of an issue to fix some places in the code (we don't use skew that much )

 

Thanks again

Tom.

[Wix Wow!Team]

BTW - I think some warning or error message on bad value types could also be useful

[GreenSock]

12 hours ago, Wix Wow!Team said:

If you are saying that the previous behaviour is limited to a limited group of cases, that it is less of an issue to fix some places in the code (we don't use skew that much )

Yeah, I'm saying it shouldn't have ever work...but you stumbled on a case where it happened to work. 

 

10 hours ago, Wix Wow!Team said:

BTW - I think some warning or error message on bad value types could also be useful

Wouldn't that necessitate validation code though? And that's one of the things I was trying to avoid. If for EVERY property of EVERY tween, we had to add conditional logic checks to look for invalid values (which, again, could be quite involved depending on what you mean by "invalid"), it adds not only kb but also slows things down. Nobody would notice most likely unless they're doing thousands of simultaneous tweens, of course, but I'm a performance nut. 

 

It seems to me like a good compromise is to simply ignore CSS tweens to undefined. Agreed? Minimal code to add, very little performance cost. Agreed?