Scrolltrigger killAll is buggy?

[Basilico]

As the codepen shows, the Scrolltrigger.killAll method leaves some instances behind.
This is not happening if I use a loop based on the getAll() like this one:

 

 ScrollTrigger.getAll(t => t.kill()) 

 

See the Pen WNyVXWm by fnool (@fnool) on CodePen

[Cassie]

Interesting, thanks for this really clear demo!

paging @GreenSock...

[Cassie]

FYI for Jack - It's seemingly only killing every other ScrollTrigger - I rolled back to 3.11 and it's the same - I tried adding a ST to a tween too to see if that was different than using create

See the Pen KKeOQrQ?editors=0011 by GreenSock (@GreenSock) on CodePen

[Basilico]

While I was working on a project I did some test logging the ScrollTrigger.killAll internal loop and I found that the forEach loop simply stops earlier.
Even avoiding the "ScrollSmoother" test and just killing every "t", the loops ends before the end.
And I found it really strange!

 

_triggers.forEach(function (t) {

return t.vars.id !== "ScrollSmoother" && t.kill();

});

But if I get the triggers with getAll and then loop them, it works.

 

[GreenSock]

Thanks for reporting that, @Basilico! Sorry about the hassle. It should be resolved in the next release which you can preview at 
https://assets.codepen.io/16327/ScrollTrigger.min.js

 

Better? 

[Basilico]

Yes! Thanks!

 

Do you mind explain me the problem? I'm just very curious and the source code is not published yet.

[GreenSock]

Sure. Internally, ScrollTrigger maintains an Array of all the instances. When you call kill() on a particular instance, it removes itself from that Array. I mistakenly thought that a .forEach() would not be affected the way that a normal for... loop would be. For example: 

let triggers = [0,1,2,3],
    length = triggers.length;

for (let i = 0; i < length; i++) {
  triggers.splice(i, 1); // leads to skipping because the index shifts
}

The first time through the loop, i is 0, so it'd remove that...but now what WAS in the second slot is now in the first slot. When we iterate i in the loop, now the second time through the loop i is 1 but that is actually referring to what WAS in the 3rd slot! See the issue? For some reason, I believed .forEach() made sure that every element went through the loop rather than being index-dependent like the normal for... loop. 

 

So the solution is very simple: 

// old: 
triggers.forEach(...);
                 
// new:
triggers.slice(0).forEach(...);

Does that clear things up? 

[Basilico]

It's crystal clear now!
I also believed that a forEach was a safe way for doing that and so I didn't investigate in that direction.

I will remember it now.

Thanks!