Uncaught TypeError: Cannot set property '_next' of null

[rodneyrehm]

Hey there,

thanks for this great library! I'm using Tweenable to run a custom variation of Vue's List Move Transitions and Staggering List Transitions.

I just tried updating from 2.3.1 to 2.6.0 and now see the following errors:

Uncaught TypeError: Cannot set property '_next' of null

traced to

shifty/src/tweenable.js

Line 196 in 057fc04

previousTween._next = nextTween;

Uncaught TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined

traced to

shifty/src/tweenable.js

Line 231 in 057fc04

for (const filterType of this._filters) {

Uncaught TypeError: u is not a function

traced to (after avoiding previous error)

shifty/src/tweenable.js

Line 113 in 057fc04

_step(_currentState, _attachment, offset);

---

I applied the following patch locally to prevent above errors from being thrown, but I don't know if this would cause problems elsewhere:

@@ -82,7 +82,9 @@
   const hasEnded = timeToCompute >= endTime
   const offset = _duration - (endTime - timeToCompute)

-  if (hasEnded) {
+  if (!tween._filters) {
+    tween.stop(true)
+  } else if (hasEnded) {
     _step(_targetState, _attachment, offset)
     tween.stop(true)
   } else {
@@ -189,7 +191,7 @@
   } else if (tween === listTail) {
     listTail = tween._previous
     listTail._next = null
-  } else {
+  } else if (tween._previous) {
     const previousTween = tween._previous
     const nextTween = tween._next

I'm happy to provide a PR for this if you consider the fix sufficient (and you're not quicker at fixing this yourself)

[jeremyckahn]

Hi @rodneyrehm, thanks so much for this great bug report, it's really helpful for diagnosing the issue! It looks like the null reference error is a result of the somewhat recent internal change to a linked list for animation processing that was introduced for 2.5.0. It was a significant change that I tested as best as I could, but perhaps something was overlooked! 😅

The stack trace you provided is a bit concerning to me, as tween._filters should always be defined an array and never falsy. So, the for...of on line 231 should never fail as it is doing here. Since you ran into that reference error and had to handle that scenario as a special case, it suggests to me that Tweenable is being put into a state that it wasn't meant to be. Would you be able to provide a bit of sample code to reproduce the issue so I could debug it a little further?

[rodneyrehm]

I could provide the vue component if that helps you, otherwise the significant bits are:

to execute the transition I run

// create new tweenable, unless element already has an instance
const tweenable = new Tweenable()

if (updateWhileTranisitioning) {
  // abort previous mutation (rejecting previous promise)
  tweenable.stop(false)
}

const tween = tweenable.tween({
  from: _from,
  to: _to,
  duration,
  delay,
  easing,
  step: _update,
  start: _update,
})

tween.then(finishMutation).catch(abortedState => {
  // ignore rejection caused by calling .stop(false) on the mutation
})

when a transition finished I run

finishMutation () {
  tweenable.stop(true)
  tweenable.dispose()
}

to perform the transition I run (variations of)

const _update = state => {
  element.style.transform = `translate3d(0px, ${state.y}%, 0px)`
  element.style.opacity = state.opacity
}

[rodneyrehm]

The error Uncaught TypeError: Cannot set property '_next' of null suggests the remove operation sometimes can't properly remove an item from the linked list, if the tween doesn't know it's previous element in the list anymore. (no idea how that happens, though, maybe caused by dispose() being called?) That would make the item we're trying to remove stick around in the _next reference of some other tween, causing the scheduled update to keep trying to access a destroyed tweenable instance. That would explain why I get thousands of those Uncaught TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined errors logged.

Is there a reason you went with a double-linked-list instead of going with a simple Set? The naive me assumes the order in which the update steps of concurrent tweens are performed doesn't matter, so a Set would give you fast lookup and iteration, while not having to deal with your own data structure.

[jeremyckahn]

Thank you for posting the sample code! Without being able to run it as posted above, this appears to be a pretty straightforward use case of Shifty. The one thing that is suspect to me is the call to tweenable.dispose(). What happens when you remove that? It is not strictly necessary to call dispose(); it's mainly there for the odd times where a degree of manual memory management is desired. It doesn't seem that this scenario would require dispose(), so removing that might be the simplest fix.

---

Is there a reason you went with a double-linked-list instead of going with a simple Set?

To be honest, I don't recall giving that option much consideration when I was designing the batched tween processing system. Set seems like it would be a practical alternative to the doubly-linked-list system that's there currently, but I'd want to understand it a bit more before forming an opinion on it as I've never used Set before. My immediate concerns are slightly spotty IE support (though that probably wouldn't be a real issue for how Shifty would use it), and performance implications. This concerns a part of the library where performance is a top priority.

I'm open to exploring that more in a separate Github issue if you think it would be worth changing to!

[rodneyrehm]

What happens when you remove that? It is not strictly necessary to call dispose()

I still see the Uncaught TypeError: Cannot set property '_next' of null error when an element is supposed to be removed. But now I know why…

Once the promise returned by tweenable.tween(…) resolved you cannot call tweenable.stop(true) anymore, without getting that _next of null error. Because I wasn't handling this situation, my own mapping of element/tweenable wasn't cleaned up properly, causing me to re-use tweenable instances that had already been dereferenced (from the linked list) and destroyed.

After removing the tweenable calls from my finishMutation() handler I'm not seeing any problems anymore. Maybe the docs could mention that, so the next person doesn't fall into the same trap? (or maybe it's just me, who knows…).

Anyway, thank you very much for all your work! :)

I'm open to exploring that more in a separate Github issue if you think it would be worth changing to!

Since you already have that implementation I don't think you need to worry about this. I don't think there would be major gains by switching to Set (other than maybe simplifying your code a bit).

[jeremyckahn]

Once the promise returned by tweenable.tween(…) resolved you cannot call tweenable.stop(true) anymore, without getting that _next of null error.

Ahhh I think I see what's happening here now! I think that stop should handle this scenario a little more gracefully. An extra check or two in the internal remove method should be all that's necessary, but I'm going to reopen this and dig into the issue a bit more deeply as I have time to.

Thanks for all the info — I will update this issue once I've made some progress on this!

[jeremyckahn]

This is definitely a bug worth fixing. Here's a simplified reproduction of the issue: https://codepen.io/jeremyckahn/pen/qQmQJJ

The issue happens when you call stop() more than once on a Tweenable instance when there is more than one tween currently running. I'll prioritize a fix!

[jeremyckahn]

This is fixed in 2.6.1: https://codepen.io/jeremyckahn/pen/Zmyeyo

Please give it a try. Thanks again for reporting and helping to track down this issue, @rodneyrehm!