titanium-sdk
titanium-sdk copied to clipboard
chore(test): add tests for Promise
JIRA: https://jira.appcelerator.org/browse/TIMOB-27483
Execution order for Promise
is broken on iOS
MWE:
Promise.resolve()
.then(() => console.log('A'));
console.log('B');
Correct output:
B
A
My guess is that jobs queue is drained after call to non-js-native methods (?).
Tests with names like does not break execution order...
are same as follows execution order...
ones, except they have console.log
calls.
UPD: in my tests it works correctly on ios 9 (9.3.5) and fail on ios 10+ (10.3.1, 12.1.1, 12.3.1, 13.1.2)
Warnings | |
---|---|
:warning: | This PR has milestone set to 10.0.0, but the version defined in package.json is 10.2.0 Please either: - Update the milestone on the PR - Update the version in package.json - Hold the PR to be merged later after a release and version bump on this branch |
Messages | |
---|---|
:book: | :fist: The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready. |
:book: |
:white_check_mark: All tests are passing Nice one! All 19694 tests are passing. (There are 1157 skipped tests not included in that total) |
:book: | :tada: Another contribution from our awesome community member, drauggres! Thanks again for helping us make Titanium SDK better. :thumbsup: |
:book: |
:floppy_disk: Here's the generated SDK zipfile. |
Generated by :no_entry_sign: dangerJS against d492cac846bfa76add20e8484290f30a6fb8d6ee
@drauggres Can you create / link a JIRA ticket for this? It looks quite critical, but to schedule it, I think Appc needs a ticket to track it. And does this also apply for async await
blocks?
Can you create / link a JIRA ticket for this?
Done
And does this also apply for
async await
blocks?
It does: async/await
blocks are transpiled into Promise
s
So my initial suspicion was that this was caused by the promise polyfill stuff. But even if I run on an iOS 13 sim and disable Promise polyfills (to use the native implementation) we see this issue.
- I replaced the
console.log()
calls withTi.API.info()
calls, same issue. - Replace them with simple variable assignments, it works.
- Replace with Ti.App.* property accesses, fails.
- Replace with a for loop the just does some assignments/math - it works.
- place a
console.log()
call in the top-level before appending to theresult
variable, but have none in the Promisethen()
implementation - fails. - Have none in top-level, but place one before modifying
result
in the Promisethen
function - works.
So it certainly looks like crossing the "bridge" has some impact on deferred things like Promises. If we hit a call to a Ti API in top-level/current context, it seems to first allow any scheduled Promises to fire first.
So let me try to illustrate what we should get versus what's happening:
let result = '';
Promise.resolve()
.then(() => {
result += '2';
})
.then(() => {
result += '3';
});
console.log('1');
result += '1';
In this example, result
should eventually be '123'
, but is instead '231'
. The call to console.log('1');
is basically letting the promise fire off. In my testing, a call to any of the Ti
namespace methods does the same. So this is a pretty low-level issue in terms of how we're hooking to the JS engine.
I still can reproduce this issue on master
(so here is also a question about the testing environment).
I think this is really critical issue, because it is breaking code execution order.
(Same as if 2+2*2
were returning 8
on iOS and 6
on Android)
@drauggres Do you have a suggestion to move forward here? Happy to merge the new tests, but if I followed correctly, they focus on the example that iOS behaves differently, not on the fix so far. I can also try to take a look here, but I'm unsure that will be easy to fix.
Initially, these tests failed on iOS.
I don't see testing in the github actions now, but the previous build (d492cac
commit) was done with jenkins and should have been tested. It is maked as successful, i.e. the problem was fixed (I don't know how or when).
Thats odd. @sgtcoolguy Do you know by any chance if there was a change here last year fixing that issue?
EDIT: Unfortunately not fixed. The following still returns A B
:
Promise.resolve()
.then(() => console.log('A'));
console.log('B');