-
-
Notifications
You must be signed in to change notification settings - Fork 753
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: resolve a new Promise #2450
Comments
Is that new behaviour? I always assumed that when you resolved a promise, you resolved with the value you wanted to return. So if you resolve but pass another promise as the value that you're resolving, that then gets added to the chain? |
I can't face trying to fix this at the moment, so I've just made it throw an exception if you try and resolve with a promise so at least you know it's not doing what you expect. |
I am not sure if its new or not. But its based on the distinction between I don't mind implementing it. |
Thanks! Yes, it'd be great if you could fix it - but ideally the less code you end up changing in the process the better. Hopefully it can be done by just linking the new promise in in the right place |
Hey, I am about to test some code regarding this, but I want to test on the emulator instead of flash to my watch so often. Do you have some tips how I can update the emulator to use the firmware I push via github? Ah it seems I can just run the linux version? Because I am testing non-bangle feature. |
Yes, totally! That's the best way to do it. It has some promise tests built in already - and will be able to check for memory leaks too |
I will intend to fix this bug too, its related to .then() not always returning a new promise, when it needs to because they can spawn independent chains. (function() {
p1 = Promise.resolve();
let p2 = p1.then(()=>{
return new Promise((y,n)=>{
setTimeout(()=>{
console.log("hm");
y("hm");
},1000);
});
});
let p3 = p1.then(()=>{
return new Promise((y,n)=>{
setTimeout(()=>console.log("3"),4000);
});
});
//p2 resolves first, but both p2 and p3 fire.
p2.then((y,n)=> {
return new Promise((y,n)=>{
setTimeout(()=>console.log("p2then1"),1000);
});
});
p3.then((y,n)=> {
return new Promise((y,n)=>{
setTimeout(()=>console.log("p2then2"),1000);
});
});
console.log(p2===p3);
})(); Node output:
Espruino output:
|
It depends how entwined they are I guess. If they're reasonably separate sets of changes then two PRs would be good so it's easier to track down if it turns out something breaks later |
Ok, I will make separate ones for each issue I discover, (if) I find more. |
Thanks! Just a note but there's #2227 which I think is the issue you described above |
Good find, yes that is the same thing and described well. Out of curiosity what was the reasoning for using obj.chain.chain.chain? I have a feeling it can be achieved without chain, but don't stress yourself in remembering, I will try to take care of this. |
I have fully researched how other implementations can have the correct behaviour without a chain. The secret is to |
I have made some progress, had to change quite a lot of the code to get the behavior I wanted. However, my weakness stems from not understanding the purpose and correct usage of Is there something I can research to understand it better, at the moment it seems I am unlocking everything as often as I can, without really understanding if I should be. If there is I think I need a complete Primer on garbage collection and locks. |
Say for example here, we unlock the resolv and reject callback function objects, after the function call of the executor. By unlocking them, does that mean they are free to be garbage collected? The situation when the executor uses a delayed/asynchronous call like Did passing the resolve function object into setTimeout increase its refcount? If I want to keep an object protected from garbage collection over a larger number of C function calls, would I not call I think there is a high chance that I accidently use the I would think that Mb I need to just play around with the So, if I instantly |
Honestly, it's a bit of a concern to hear you say you've rewritten most of the code for this and then asking how to locks work :) It felt like really the changes for this should have been quite minor! Promises are quite a core bit of Espruino especially on Bangle.js so I really don't want this to get broken or to suddenly use lots of memory or lose performance. On the lock front, I'm sure it was documented somewhere, but I've just tried to push a reasonably comprehensive guide to https://github.com/espruino/EspruinoDocs/blob/master/info/Internals.md#garbage-collection-reference-counts-and-locks (eventually it'll be in https://www.espruino.com/Internals) But basically:
|
If the stakes are that high, then we can make a slow release of it, fine-tuning it and reducing it down as much as possible. And testing it slowly before merging it. There is no hurry. I'm just laying down the foundations of it for now, so that it behaves correctly still. But thanks for the information I requested, it will enable me to validate my usage of locks a bit better and thus have cleaner code and less work your end. |
settledr1
is printed instantlyexpected:
outer promise settles/fulfills upon inner promise being resolved.
settledr1
is printed instantlyexpected:
settledr1 should not be printed because r2 inner promise never resolves.
The text was updated successfully, but these errors were encountered: