Resolving a promise using onComplete or Tween.then

[mikehwagz]

Howdy 🤠 Massive fan of GSAP and absolutely loving 3 so far.

While migrating a project from 2 to 3, I discovered a bit of unexpected behavior that I think would be considered a bug.

I've created a codepen to demonstrate the issue. As you can see, when attempting to resolve a promise using onComplete, resolve can no longer be passed first-class to onComplete as it could in gsap 2:

function test() {
  return new Promise(resolve => {
    gsap.to(square, {
      duration: 1.5,
      x: 200,
      ease: 'power3.inOut',
      onComplete: resolve
    })
      // also doesn't resolve using Tween.then
      // .then(resolve)
  })
}

test().then(() => {
  // never resolves
})

With that said, wrapping resolve in an arrow function (i.e. onComplete: () => resolve()) works. Another workaround is passing an empty object to onCompleteParams, which I believe is related to this line in core, but that still doesn't work with Tween.then.

Sure, there are workarounds, but because this behavior is different than that of v2, I figured it would be worth bringing to your attention! I like the simplicity of passing functions first-class, so it would be great to be able to continue doing so with gsap 3.

Just in case there is curiosity regarding the use-case here, I am working with a 3rd party lib (highway.js) that passes a done callback to Transition.in and Transition.out. After inspecting the source, I realized that done is actually just resolve coming from a Promise, which led me to creating these isolated tests in codepen.

Thanks for your prolific work on GSAP! Cheers 🍻

[ZachSaucier]

Hey Mike, thanks for posting.

In GSAP 3, there's a new method on Tweens and Timelines called [.then()](https://greensock.com/docs/v3/GSAP/Timeline/then()) that returns a promise. You can use it like so:

function test1() {
    return gsap.to(square, {
        duration: 1.5,
        x: 200,
        ease: 'power3.inOut'
    })
}

test1().then(() => {
    // resolves!
    console.log('test1 done')
    square.style.backgroundColor = 'green'
})

Jack may comment addressing your concerns but I thought that might be helpful information in the mean time.

Thanks for the reduced demo and workarounds!

[Danetag]

It seems that using onFulfilled instead on onComplete does the trick. There's something weird with the then() method exposed by gsap...

When this works

return new Promise(async resolve => {
        const test = await gsap.to(triangle, {
            duration: 1.5,
            x: 200,
            ease: 'power3.inOut',
        }).then()

        resolve();
    })

This doesn't

return new Promise(async resolve => {
        const test = await gsap.to(triangle, {
            duration: 1.5,
            x: 200,
            ease: 'power3.inOut',
        })

       // doesn't come here
        resolve();
    })

[mikehwagz]

@ZachSaucier thanks for the feedback! I'm aware of the new promise returning .then() method in gsap 3 and I have been taking advantage of this new feature in other cases.

However, this particular issue is that passing a resolve function first-class to onComplete no longer works as it did in gsap 2 (and in JavaScript in general).

I would not typically pass resolve to onComplete or .then(), but in this case, resolve is being passed by a 3rd party lib, so although my codepen example might seem a bit convoluted, it does demonstrate the inability to call resolve first-class.

[ZachSaucier]

Potentially related to this issue in the separate (and hopefully obsolete soon) gsap-then library. It'd be good to remove the need for this sort of thing and have it all built in 🙂

[Danetag]

I think there's one "real" issue: passing a resolve function to the onComplete property only works if you give an object to the onCompleteParams property so it would do callback.apply() instead of callback.call() (see here)

The then() is mostly confusing, because it implies gsap.to() returns a Promise... Maybe changing the name of this would help?

[OSUblake]

The then() is mostly confusing, because it implies gsap.to() returns a Promise... Maybe changing the name of this would help?

It looks like the name will need to change for it to work with await. Do you have a suggestion?

[OSUblake]

I just tested, and await works if then is changed to a different a name. This should be fixed, and is going to be breaking change.

Node.js has a promisify util. https://nodejs.org/api/util.html#util_util_promisify_original

What if we did something like that?

gsap.utils.promisify(gsap.to(foo, { x: 100 }));

Or change then to promisify.

gsap.to(foo, { x: 100 }).promisify();

Or maybe have both, a promisify util function, and a promisify method.

[jackdoyle]

Interesting. I'm a little fuzzy on why the "then()" name is the root problem, and simply changing that to some other name resolves it (get it? "resolves" It...I crack myself up). Any insight on that?

[OSUblake]

@ZachSaucier linked to an issue that kind of describe the problem. Promises have a then method, and so do gsap animations. await sees the then method, and thinks the animation is a promise, so it waits for it to resolve, which it won't.

[jackdoyle]

NOT passing "this" (the Tween/Timeline) to the onFulfill method also seems to fix things. @OSUblake I can't remember right now, but I think you requested that it be passed that way originally and there was good reason - do you remember? From what I can find online, it seems like onFulfill is just a simple callback, so there isn't a need to pass it the Tween/Timeline reference...am I missing something?

[OSUblake]

I can't remember right now, but I think you requested that it be passed that way originally and there was good reason - do you remember?

When resolving a promise, you can pass in the value so that it can be used in the fulfilled function. For example, see how I'm passing in the image to resolve so that it can be used in the then function. https://codepen.io/osublake/pen/868aa02926ff4ee26946d571176b25fc

I thought it might be helpful if the animation was passed just in case someone needed to access the animation for whatever reason.

But, the problem with not being to call resolve in onComplete goes back to the then method name. Change it to something else, like promisify, and it works fine.

[OSUblake]

Moral of the story. Having a method named then is a very bad idea.

[OSUblake]

Working demo. https://codepen.io/osublake/pen/9dd5777926585e418231c13ad755dbee

[jackdoyle]

Another solution (which wouldn't cause a breaking change, so I like it) is to check if the animation has already completed when then() is called, and if so, immediately invoke resolve(). I just implemented that and pushed an update here and it seems to work great: https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/gsap-latest-beta.min.js

Any objections?

[OSUblake]

You're not passing in this? I would use the resolved animation to get data or the id from the animation.

But I don't think your solution works correctly. This demo should display the console log messages immediately. It's waiting for the animation to finish, which is incorrect. https://codepen.io/osublake/pen/50d83ada3767b54f10ce4e4667cd1417

[OSUblake]

If you're getting confused by all the await stuff, this is how normal promise behavior should behave. You can put any value into Promise.resolve(). If it's a promise, it will wait for it to resolve, otherwise it will resolve immediately. When it resolves, it passes the value into the next then function.

Promise.resolve(3).then(value => {
  console.log("Resolved number", value);
});

[OSUblake]

If everything is working correctly...

This should resolve immediately, and log out the tween.

Promise.resolve(gsap.to({}, { duration: 2 })).then(value => {
  console.log("Resolved tween 1", value);
});

This should resolve after 2 seconds, and log out the tween.

Promise.resolve(gsap.to({}, { duration: 2 }).then()).then(value => {
  console.log("Resolved tween 2", value);
});

But that doesn't happen. I think a breaking change might be the only option. I would change the name to promisify, and get rid of the onFulfilled part.

gsap.core.Animation.prototype.promisify = function() {
  var _this = this;

  return new Promise(function (resolve) {
    _this._prom = function () {
      resolve(_this);
    };
  });
};

So the new usage would be like this.

gsap.to({}, { duration: 2 }).promisify().then(value => {
  console.log("Resolved tween", value);
});

// resolves after 3 seconds
Promise.all([
  gsap.to({}, { duration: 1 }).promisify(),
  gsap.to({}, { duration: 2 }).promisify(),
  gsap.to({}, { duration: 3 }).promisify()
]).then(tweens => {
  console.log("Resolved tweens", tweens);
});

// maybe have a util method to promisify animations?
gsap.utils.promisify([tween1, tween2, tween3]);

// would return this
[
  tween1.promisify(),
  tween2.promisify(),
  tween3.promisify()
]

// or this
Promise.all([
  tween1.promisify(),
  tween2.promisify(),
  tween3.promisify()
]);

// so you can do something like this
gsap.utils.promisify(tweens).then(...)

[OSUblake]

But I don't think your solution works correctly. This demo should display the console log messages immediately. It's waiting for the animation to finish, which is incorrect. https://codepen.io/osublake/pen/50d83ada3767b54f10ce4e4667cd1417

But maybe people would be fine with that behavior??? They just have to understand that the animation will be undefined when the promise resolves.

One nice benefit is that you don't have to do animation.then() for something like this.

// resolves after image is loaded, and animations have finished
Promise.all([
  loadImage("foo.png"),
  gsap.to({}, {duration: 1}),
  gsap.to({}, {duration: 2}),
  gsap.to({}, {duration: 3}),
]).then(value => {
  console.log("Resolved", value); // [image, undefined, undefined, undefined]
})

[mikehwagz]

Personally, I'm really liking the .promisify() solution. Not only does it solve my specific case, but it also feels like a more clear/explicit way to opt-in to promises with gsap.

[fregante]

Moral of the story. Having a method named then is a very bad idea.

I disagree. The only problem with the current situation is that .then() resolves with the Tween, which itself has a .then property, so await tween goes in a loop.

The solution should be simple: don't return resolve with this.

I'd much rather be able to await any tween than having to call a method to get a promise. Without a .then, await tween will silently fail because the tween won't be awaited at all, it will just pass instantly. This can cause confusion.

I don't think I've ever encountered a Promised object that returns itself, so don't shoot yourself in the foot by thinking in terms of GSAP (whose onComplete callback passes the tween so you expect promises to behave the same)

---

I had this issue in my own gsap-then module and solved it first by adding a method (exactly like you're suggesting with .promisify) and then realized the solution was to just skip this

There are some caveats to turning Tweens into Promises, but it's doable.

[fregante]

@mikehwagz Also, regarding the original issue: you don't need any of that now since GSAP v3 is already promisified.

function test() {
  return gsap.to(square, {
      duration: 1.5,
      x: 200,
      ease: 'power3.inOut'
  })
}

test().then(() => {
    //resolves!
})

[Danetag]

@fregante but not returning this wouldn't prevent from chaining tweens?

[fregante]

@fregante but not returning this wouldn't prevent from chaining tweens?

Again you're thinking in terms of GSAP, not promises. If you use Promises, you're not chaining tweens, you're chaining promises.

// Before:
tl
    .to(el, {duration: 1.5, x: 200})
    .to(el, {duration: 1.5, y: 200});

// After
await gsap.to(el, {duration: 1.5, x: 200});
await gsap.to(el, {duration: 1.5, y: 200});

[ZachSaucier]

// After
await gsap.to(el, {duration: 1.5, x: 200});
await gsap.to(el, {duration: 1.5, y: 200});

Which would make things like negative relative offsets impossible to do 🙂

[fregante]

To clarify, to from fromTo still return this. But then (or await) doesn't. You leave the GSAP/Timeline context

[OSUblake]

The solution should be simple: don't resolve with this.

The problem with that is you may need the animation after it resolves to do something else, like with the targets or animation data.

getAnimations().then(animations => {
  animations.forEach(animation => doSomething(animation.targets());
});

There are ways around that, but it won't be as clean.

[fregante]

If then is gone, you can't do that, you'll have to call .promisify first and it won't be as clean as you wish.

If you have the Promise, you also have the original Tween. Use await:

const animations = getAnimations();
await animations;
animations.forEach(animation => doSomething(animation.targets());

The only case where this gets slightly more verbose is when you write them inline in Promise.all, e.g.

await Promise.all([
    TweenLite.to(...), // Lost
    loadImage('img.jpg')
]);

console.log('Animation done and image loaded');

So you'd have to save it to a variable first:

const tween = TweenLite.to(...);
await Promise.all([
    tween,
    loadImage('img.jpg')
]);

console.log('Animation done and image loaded', tween);

But I'd argue that most times you don't need to access such Tweens or, if you do, aren't as simple as a single tween, so you won't write them inline.

[mikehwagz]

@fregante Thanks. As I already explained, I am 100% clear on how promises currently work with gsap 3. my codepen example is obviously convoluted but it does demonstrate the inability to pass resolve first-class to onComplete. To clarify, my real world use-case where I originally encountered this issue was with the Highway.Transition class which passes a done callback to in and out methods which is just resolve internally. Take a look at this simple fade transition example from the Highway website using gsap 2. This example no longer works after migrating to gsap 3 since done (aka resolve) is being passed first-class to onComplete. To me this feels like unexpected behavior that really tripped me up.

[mikehwagz]

Also @fregante, I'm not sure it is safe to assume that if you have the promise, you also have the original tween. for example:

// some-file.js
import someAsyncFunction from './somewhere-else.js'

const tween = gsap.to(...);
tween.then(someAsyncFunction)

// somewhere-else.js
async function someAsyncFunction(tween) {
  // needs access to the tween!!
}

export default someAsyncFunction

[fregante]

it does demonstrate the inability to pass resolve first-class to onComplete

Thanks for clearing that up, I hadn't investigated that problem specifically. But yes, GSAP trips that pattern because:

1. onComplete calls resolve with tween
2. The Promise resolves with tween
3. The Promise finds the then on tween and calls it (Promises never resolve with a Promise but try to find the last resolution)
4. The Promise returned by then resolves with tween (same as step 2)
5. Repeat

When tween.then is updated to not resolve with tween, then step 4 will be:

4. then resolves with undefined
5. The whole Promise is resolved with undefined
6. Done.

[fregante]

I'm not sure it is safe to assume that if you have the promise, you also have the original tween

You were almost onto something, but:

// some-file.js
import someAsyncFunction from './somewhere-else.js'

const tween = gsap.to(...);
await tween;
someAsyncFunction(tween);

Raw .then calls are almost never cleaner than await so they should be avoided. In my example you just need to call it from inside an async function.

If you must use .then:

// some-file.js
import someAsyncFunction from './somewhere-else.js'

const tween = gsap.to(...);
tween.then(() => someAsyncFunction(tween));

[fregante]

You did make me notice a real issue though: async functions can't return tweens

async function test() {
    return gsap.to(...);
}

const foo = test(); // foo is a Promise that resolves with undefined, not the tween
const tween = await foo; // tween is undefined

Calling an async function, without even awaiting it, will already start the recursive Promise resolution, which means that it will call tween.then, which then resolves to undefined, which then is returned by test

Now that is an issue.

[thierrymichel]

Hi all,

First of all, thanks for the great hard work ! Happy to see the evolution from AS3 to ES6 😛

I'm having the same problem as @mikehwagz (but with barba.js 😁) and I'm also aware about the new possibilities based on promise…

1. onComplete calls resolve with tween

This was not in the previous version. Is this intentional? May I ask … why?

In barba.js, we allow people to use this.async() "pattern" to manage callback-based code. This is a common use case :

anime(t) {
  const done = this.async();
  gsap.to(t, { onComplete: done });
}

As it's "inspired" by the node.js world, expected params will be error and value (not tween).

https://nodejs.org/en/knowledge/errors/what-are-the-error-conventions/

When the callback function is invoked, the first parameter is reserved for an error that may have occurred. The value will be null (falsy) if no error occurred and an instance of Error (truthy) if an error did occur.

Anyway, I do not have strong opinion about it. I do not get the point but, as it's a major version, I understand that behaviors can change. It would be nice to update documentation to mention this tween parameter.

[jackdoyle]

Alright, folks, I think I've got a solution that addresses all the concerns. I've implemented it in this file: https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/gsap-latest-beta.min.js

When resolving, it temporarily nulls the then() method on that instance to avoid the infinite loop. This gives us all the benefits of then() and none of the breaking changes, plus it avoids the need to rename the method. Seems to work fine with async/await and it solves the problem with onComplete. Please let me know if it works well for you.

[fregante]

That sounds like the perfect compromise. I was gonna suggest it but I didn’t know how feasible it was. Glad you figured it out 😍

[OSUblake]

Seems to work great!

[fregante]

I think this issue still remains though: https://github.com/greensock/GSAP/issues/322#issuecomment-554045850

The workaround to that, if Tween#then exists, is to wrap the tween before returning it, so async doesn't see it:

async function test() {
    return [gsap.to(...)];
}

const foo = test(); // foo is a Promise that resolves with [tween]
const [tween] = await foo; // tween is the tween 🎉

Which is kind of awkward. Now it's up to you to decide whether this specific case ("async functions can't return a tween") is a reason good enough to avoid then.

[thierrymichel]

On my side, onComplete still using Tween as parameter… :/

// Still resolving with Tween…
onComplete(tween) {
  console.info('leave:onComplete', tween);
  done();
},

// Throws an error -> cb(err, value) pattern
onComplete: done,

[mikehwagz]

@fregante As far as I can tell, @jackdoyle's latest solution actually fixes the issue you pointed out in #322 (comment).

Here's another codepen to demonstrate as well as the code for this example below for convenience/context:

// loaded beta including latest solution from @jackdoyle
// https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/gsap-latest-beta.min.js

const square = document.querySelector('.square')

test()

async function test() {
    let tween = await init()
    console.log('resolved', tween)
    square.style.backgroundColor = 'green'
}

async function init() {
    return gsap.to(square, {
        duration: 1.5,
        x: 200,
        ease: 'power3.inOut'
    })
}

[fregante]

On my side, onComplete still using Tween as parameter… :/

@ThierryMichel No need to use that, barba accepts promises (from what I can tell):

anime(t) {
  return gsap.to(...);
}

this.async() is only a helper for animating tools that do not return Promises, like GSAP 1-2. GSAP 3 is different and requires a different approach.

[fregante]

@fregante As far as I can tell, @jackdoyle's latest solution actually fixes the issue you pointed out in #322 (comment).

D'oh, yes. I was thinking that then was still there on the second call but it resolved to undefined instead of tween (instead it's not there at all). Disregard. 😅

[jackdoyle]

on my side, onComplete still using Tween as parameter… :/

// Still resolving with Tween…
onComplete(tween) {
  console.info('leave:onComplete', tween);
  done();
},

Are you suggesting that we NOT pass the tween instance as a parameter to onFulfilled()? I thought we were all on the same page about the benefits of doing so now that we've got a workaround in place to avoid the circular then() calls. Did I misunderstand?

Or are you just suggesting that we not pass the tween instance to the onComplete when no onCompleteParams are defined because it's causing an issue in your scenario (totally unrelated to then() or onFulfilled())?

[OSUblake]

Or are you just suggesting that we not pass the tween instance to the onComplete when no onCompleteParams are defined because it's causing an issue in your scenario (totally unrelated to then() or onFulfilled())?

Yes. If someone needs the instance, they can use this with a regular function, right?

[thierrymichel]

@jackdoyle Yes. In previous versions, onComplete had no parameter… And as @OSUblake said, I expect to have the instance as the context of a regular function. Not as a parameter.

[thierrymichel]

@thierrymichel No need to use that, barba accepts promises

@fregante Haha, thanks a lot! 😇 I am the author of the v2 of barba.js which, obviously, supports promises but not only… 😛

[jackdoyle]

Yes. If someone needs the instance, they can use this with a regular function, right?

Yes, but not if they use an arrow function because those handle scope differently (unrelated to GSAP). That's why I added the animation as a default parameter - I figured people would really appreciate the convenience with arrow functions.

Would ya'all still vote against it? I don't have a strong attachment to it - I just thought folks would appreciate it.

[OSUblake]

I would get rid of it. I honestly didn't even know that it was being passed in until this issue was brought up.

[thierrymichel]

IMHO, it is inconsistent to put it ONLY IF there is NO onCompleteParams…

If you think that accessing the instance through a parameter is convenient for the users, the Tween should always be present. What happens if I want the Tween and extra params with onCompleteParams? There are so many possible scenarios…

But again, no strong opinion. v2 -> v3 means breaking changes 😊 Regarding barba.js, as v2 is not yet publicly released, I will update all examples using gsap with return gsap.to().then() and we will be fine! 😁

Just let us know.

[jackdoyle]

Okay, I can remove it and then if someone needs to reference the tween, they'll just need to use a normal function, not an arrow function. Any objections?

[jackdoyle]

Should be resolved in the latest release (3.0.2)