Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promises run only after closing the window #121

Open
wstam88 opened this issue Jun 6, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@wstam88
Copy link

commented Jun 6, 2019

For some reason promises do not resolve until the window is closed.

For example when adding this piece of code just before win.showAll(); at examples/builder.js.

let myPromise = function(){
    return new Promise((resolve, reject) => {
        resolve('Hi...');
        console.log('This gets printed just fine...')
    });
}

myPromise().then(res => {
    console.log('this only runs after closing/exit the window');
    label.setText(res);
});

Doesn't really matter where I place it, also happens inside the body of an event callback function for example.

@romgrk romgrk added the bug label Jun 6, 2019

@romgrk

This comment has been minimized.

Copy link
Owner

commented Jun 6, 2019

Ok, this is weird.

So normally we're already handling micro-tasks here, so promises should be called even when GLib's loop is running (Gtk.main()).

And now, if I add a timeout (that doesn't run for a long time), the promise handler is called. (wtf)

let myPromise = function() {
  return new Promise((resolve) => {
    resolve('Hi...')
    console.log('This gets printed just fine...')
  })
}

myPromise().then(res => {
  console.log('this only runs after closing/exit the window')
  label.setText(res)
})

setTimeout(() => {
  console.log('timeout')
}, 999999)

I'll investigate this.

@wstam88

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

Well if you put it in an interval it does work! And the other ones outside the interval suddenly also work 🤔

setInterval(() => {
    myPromise().then(res => {
        console.log('works...')
    });
}, 1000)
@wstam88

This comment has been minimized.

Copy link
Author

commented Jun 6, 2019

Sorry didn't understood your comment correctly...

@subjectdenied

This comment has been minimized.

Copy link

commented Jun 6, 2019

i'm not sure we mean the same, but inside event-bodies i suceeded by using nodejs setImmediate

@wstam88

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

I noticed that as well, but it doesn't really run immediately or consistently. Sometimes it takes a few seconds and sometimes it just skips.

As soon as originalMain() is called it completely blocks.

The promise status is resolved and the value is there, it's just the callback that gets stuck in the eventloop/next_tick. When originalMain() is stopped then suddenly every promise resolve callback is called.

Also setTimeout inside an event callback doesn't run until closed.

btn2.on('clicked', () => {
    // this runs, and promise is resolved
    let promy = prom();

    let timeout = setTimeout(() => {
        console.log('this does not run...')
    }, 1000);

    debugger

    // it's just the callback that gets stuck
    promy.then((val) => {
        // this block runs after closing the Gtk window
        console.log(val);
    });
});

example

@romgrk

This comment has been minimized.

Copy link
Owner

commented Jun 7, 2019

I've made things work as expected with #122 (note my skillful avoidance of the word "fixed"), I'm waiting for the tests to complete before merging.

@romgrk romgrk closed this in #122 Jun 8, 2019

romgrk added a commit that referenced this issue Jun 8, 2019

Merge pull request #122 from romgrk/promise-issue
fix #121: promise microtasks not called
@wstam88

This comment has been minimized.

Copy link
Author

commented Jun 8, 2019

See my comment here

EDIT(romgrk): including comment:

Aah no it doesn't... It works great for simple promises like the one in the builder.js example but when you start using more complex promises like Axios it will not work as expected. It doesn't respond.

The only thing that seems to fix it is changing the interval @ placeholderIntervalID = setInterval(() => { /* noop */ }, 50)

If you turn the value to 20 seconds (20000) you will have to wait 20 seconds for the axios response.

@romgrk romgrk reopened this Jun 8, 2019

@romgrk

This comment has been minimized.

Copy link
Owner

commented Jun 13, 2019

The point of the interval was not to trigger an event-loop iteration each time it is called, but rather to keep it running by having something on it, which doesn't seem to be working. I'll try to have a minimal reproduction of the issue and will investigate in more details what's going on in libuv & GLib loops.

@romgrk

This comment has been minimized.

Copy link
Owner

commented Jun 13, 2019

I also note that for axios, if one clicks the "Action" button in the new builder.js version, the axios promise is processed. It seems nodejs doesn't notice that some microtaks have been queued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.