Objects with an "andThen" function are treated as Promises

[osyrisrblx]

Promise.is() will treat any object with an "andThen" function as a Promise object. This causes the library to call the object's andThen function even if it is not actually a Promise.

This behavior is unexpected.

[evaera]

I understand the concern with the promise library "eating" the andThen field, but the plus side is that it opens the door for multiple versions of the promise library (or even multiple, distinct promise libraries from other authors) to interoperate with a very slim contract

The shared contract the promise library holds is anything with an andThen method is a promise (or something like a promise). That's the compatibility guarantee: it means we can make large breaking changes with the promise library and maintain compatibility with all other versions of the library that uphold that contract.

There are already examples of this in the wild, where in the same game may have promise v1, v2, and v3 running at the same time. We want promises to be able to interoperate between these versions without causing problems.

I'm hesitant to change how it works because this is also how Promises work in the JavaScript ecosystem, and it may also break compatibility with older versions of the promise library.

[Dionysusnu]

A simple solution would be to provide an explicit opt-out functionality, for example by including something like:

and not rawget(objectMetatable, "PROMISE_IGNORE")

in the Promise.is implementation. Classes that use a non-Promiselike andThen method can then add this field to their metatable, to prevent the undefined behaviour.

However, this isn't perfect, because the implicit enabling of the feature can still be confusing, and take a long time to find.

[osyrisrblx]

This behavior makes sense to me, but there's no "escape hatch" to insist that your value is not a Promise. Would be nice if the existence of a very particular field (while also having an andThen function) would mark the object as not a Promise.

As it stands right now, objects with andThen functions are just not able to be used by this library.

[evaera]

@Dionysusnu @osyrisrblx Can you provide a slim example of an object with a function named andThen that doesn't work well when the Promise library calls it automatically?

[Dionysusnu]

Semi-related to this is that the existing check will fail to detect classes that extend from a Promise-like. These use a double __index, the first pointing to the extending class, which also has a metatable, which has an __index field pointing to the actual Promise-like class.

[Dionysusnu]

Can you provide a slim example of an object with a function named andThen that doesn't work well when the Promise library calls it automatically?

(in roblox-ts to demonstrate typings)

class A {
  andThen<T>(arg: T): { wrapValue: T } {
   return { wrapValue: arg }
  }
}

(simplified lua)

{
  andThen = function(arg)
    return { wrapValue = arg }
  end
}

the Promise library will call this function with a callback it expects to be executed when the Promise-like is resolved. It then returns the return value of this function, expecting it to be a Promiselike. Instead, it is a table with wrapValue being a callback, which causes undefined behaviour.

A more realistic use case of this can be found here. When used in Promise.resolve(Option.some(1)), the Promise library will chain onto the Option, thinking it's a Promise-like. The class will call the callback, expecting the callback to return an Option<T>. Instead, the callback returns T directly, which is then used as the resolved value, instead of being wrapped in the Option. Link to thread on discord involving debugging process and more detail about undefined behaviour.

[matthargett]

We have quite a few tests that create mock promises that end up interacting with the real (evaera) Promise. In some codebases, they even synthesize these sort of stub promises on purpose and codify it using a Thenable<T> or IPromise<T> interface that only defines then() (or in our/Lua case, anThen().

[Dionysusnu]

We have quite a few tests that create mock promises that end up interacting with the real (evaera) Promise. In some codebases, they even synthesize these sort of stub promises on purpose and codify it using a Thenable<T> or IPromise<T> interface that only defines then() (or in our/Lua case, anThen().

Yeah, I don't think anyone is arguing to remove this behaviour entirely. However, it's not desirable to have it forced like this, with no way to disable it.

[evaera]

After some consideration and discussion with others, we've come up with the following possible solutions:

• Simply check if the object's metatable is the same as the Promise. This breaks interoperability between multiple versions of the Promise library the same game might have, so this is a non-starter.
• As suggested earlier in this issue: an "opt-out" mechanism, which would allow library authors to say "I'm not a promise", so the Promise library doesn't call its andThen. This solution has the inherent problem that the author has to be aware of what Promises are to be able to opt-out of the behavior.
• We add a marker type to Promise, like isPromise = true, and check for it in the Promise.is function. Everything that vendors the Promise library (datastore2, roblox-ts, etc.) would then have to update to be compatible with new releases of the Promise library. Otherwise, user code will break if they use a newer version of the Promise library in their game. Updating everything and bifurcating the Promise ecosystem would be extremely painful.
• We don't do anything. The JavaScript ecosystem has this exact same problem, and that's just the way it is. Thenables, objects with then methods, are automatically invoked by native JS promises.

If we could go back in time, I think the best solution would be to add isPromise = true to the Promise.is contract. However, my intuition tells me that most Roblox developers don't often update dependencies because they don't use a package manger, so many users who are not as in-tune with our Roblox open source communities could have their games broken by just adding an additional library. We want to avoid this.

Adding an opt-out mechanism adds a burden to every other library that could ever exist, and I don't want to set the precedent of adding that technical debt to our entire community.

While the current state of things certainly causes problems, I don't think the problems are bad enough to warrant the pain or the debt that these other solutions would incur. In other words, given where we are now, leaving things the way they currently are is the lesser evil.

I don't think it's so bad, given that JavaScript, a much larger ecosystem, also has this exact problem and the things are alright.

I'm sad that we couldn't find a satisfactory solution to this problem, but I don't think there's any more discussion needed on this topic, so I'm going to close this issue.