-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
angular with tsconfig target ES2017 async/await will not work with zone.js #31730
angular with tsconfig target ES2017 async/await will not work with zone.js #31730
Comments
/FYI @domenic The short answer is that it is not possible to intercept native promises in the VM. (Native promises is used by We have two approaches to this problem:
Out of the two approaches I think second one ( This effort has been a bit on back-burner, but I will restart the discussions with @domenic. |
@mhevery , got it, I will also do some research about the v8 hooks. thank you! |
node.js begin to support https://github.com/nodejs/node-eps/blob/master/006-asynchooks-api.md so I will try to implement zone.js with @mhevery , do you think this is the correct direction? please review. |
Yes that is. I have worked with the people on async hook, and it should
work well with Zone. That means that we should be able to drop the Promise
polyfiil for node.
…On Sun, May 21, 2017 at 9:37 AM, JiaLiPassion ***@***.***> wrote:
node.js begin to support async_hooks,
https://github.com/nodejs/node-eps/blob/master/006-asynchooks-api.md
nodejs/node#13000 <nodejs/node#13000>
nodejs/node#13139 (comment)
<nodejs/node#13139 (comment)>
so I will try to implement zone.js with async_hooks in node.js, it should
have better performance, and can handle native async/await issue in
node.js.
@mhevery <https://github.com/mhevery> , do you think this is the correct
direction? please review.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/angular/zone.js/issues/740#issuecomment-302947655>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAG1T1cGv4snMB_vUhuz71k5GM8jMwWvks5r8Gg4gaJpZM4M-wuM>
.
|
@mhevery , got it, I will try to use |
@JiaLiPassion - Awesome stuff! |
@amcdnl, this is still under development, it will take some time, and I have to wait for nodejs to provide some additional information to finish this one. |
Thanks for the update @JiaLiPassion ... I was able to temp work around it by transpiling my code from ES2017 to ES2016 which sucks but is ok for short term. |
Has there been any progress on this one? As far as my limited skill of perception can see, this is the main issue that prevents angulars change detection work properly with es2017's async/await. ES2017 is realy helpfull for debugging as the compiled sourcecode by typescript/angular is not that different to the real source and so sourcemaps work much much better. Thanks for your time, Tim. |
@JiaLiPassion can you say something about the current status? |
@enko, @shellmann, this issue is still pending because there is no proper way to patch So now, sorry this issue can not be resolved, so Angular can not work with |
@JiaLiPassion Would that be https://github.com/domenic/zones? |
@enko, yes, it is, it has been in stage 0 for about two years... |
@JiaLiPassion How about node.js now? Can we resolve the question with async_hook? has examples? thx! |
@royalrover, yes, with |
Just want to point out the "obvious" workaround for Node.js, which is to downgrade target to ES2015 😄 . Thanks for your great work! |
I get it that with current state of browsers this is not possible to fix by Angular team. However, it's a huge issue going forward with Angular lacking support for not-really-that-recent ES standards. As I get it, Google uses Angular a lot internally, there is some MS involvement as well and I get also that this issue requires "political" actions, not technical ones. I hope people on Angular team understand how severe problems this little tiny issue #740 is going to cause moving forward, if those "political issues" to push browser support are not taken seriously by some more powerful entities within Google and maybe within MS as well. If those two giants were serious about the issue, it would be already fixed by now. For now, the obvious workaround for the issue is just sitting on ES2015 target forever with horrendous generated async/await code, lack of optimizations and lack of proper debugging due to that generated code. If we are just sitting and waiting "Zones for JavaScript" proposal to get anywhere from stage 0 of the TC39 process, after that was presented at the January 2016 TC39 meeting, we can be just waiting forever, without any hope about Angular ever supporting web standards going forward. You don't possess the power, but your bosses, or bosses of your bosses, or bosses of bosses of your bosses do possess the power, so please, push them hard! |
I've followed this issue for a while now of curiosity and I think it's time to share my own solution to the problem, implemented in Dexie version 2.0. It has been used for quite a while and works across all browsers keeping the zones between await calls. My first beta was released in October 2016, and the first stable 2.0 release in September 2017. Dexie has ~15k weekly downloads on npm and there are no show-stopping issues related to its zone system. The reason for having a zone system in Dexie is to keep track of ongoing transactions. First version with the zone system built-in was release in 2014 (but then without support for async/await). This was before I knew about the concept of zones, so I thought it was my own invention at first, and called it PSD (Promise-Specific Data) as it only cares about promise-based async flows. The 2.0 version use something I call zone-echoing. It will enque micro-tasks that will re-enter the zone in coming micro-tasks. It can know when it's time to stop the zone echoing as long as the user sticks to its own Promises. A fallback will stop it by a limit. This works across all modern browsers and does not leak zones (except for MutationObserver subscriptions, whose events will derive the zone from the zone where the subscription was initialized - which wouldn't be a problem in angular/zone.js as it also patches those events). The technique I use could be used in angular/zone.js but then every promise-returning function in the DOM would have to return a zone's own Promise (maybe it already does?) in order to invoke the zone echoing. There could be performance implications that would need to be considered. Especially as I detect native await calls by defining Promise.prototype.then using a getter instead of a value, and have to echo zones in order to support calls that awaits non-promises. If interested, the implementation lies in Dexie repo at src/helpers/promise.js. |
@dfahlander, thank you for the post, I will definitely read it, and I will contact you if I have any questions. |
@dfahlander, thanks for your post again, I have sent you an email, please check it. |
@dfahlander, I think I have a basic understanding of your code, I tried the same way, it works in some cases, but not work in the case below.
In this case, I think Not sure my understanding is correct, please confirm, thanks! |
That is true. Maybe the check |
@dfahlander, thanks for the response, in this case,
the return value of function |
True. Unless accompanied with a coding guideline of how to utilize zones together with native async await. |
@dfahlander, thanks for the clarification! |
Should this ticket have someone assigned on it? Now ES2019 (and ES2018) have been finalized, and Angular 8 is coming with support for producing both legacy (ES5) and modern (ES2015+) Javascript bundles. However, there is not much point of that, if modern ES versions ES2017, ES2018 and ES2019 are not supported anyway in Angular. Is there any way to overcome this problem either by fixing it somehow or at least being able to use ES2017 or ES2018 target, but somehow use non-native async/await for time being? |
Excuse my possibly stupid question, but why can't zone.js override the global |
@rvalimaki, this is a very difficult issue to resolve, I am still trying to find out a walk around. |
@JiaLiPassion thanks for clarifying |
ZoneContextManager does not propagate context when entering native async/await. open-telemetry/opentelemetry-js#1502 angular/angular#31730 Transpiling to es2015 doesn't sound great. Looking for alternatives. Maybe `Dexiejs` promise which promises to work.
I wonder if there is a way to support this with some unplugin/babel/webpack plugin which will transform the code to propagate the zone? Instead of relying on Promise being patched, the expression inside |
Would it work if transform the code with babel plugin ? click = test();
const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
const test = async () => {
let $$currentZone = Zone.current;
console.log(Zone.current.name); // will output 'angular'
await Promise.resolve(delay(100)).then((value) => {
$$currentZone = Zone.current;
return value;
});
$$currentZone.run(() => {
console.log(Zone.current.name); // will output 'root'
});
}; |
I regret to say we completely ditched zone.js and opted for explicit pass-down of the context parameter. We experienced nice performance improvements and good call stacks, and overall I'd say it was the right call. Now we're tracking this: https://github.com/tc39/proposal-async-context I think it will solve a lot of problems that we were trying to solve with zone. |
@wh1t3cAt1k not sure about your use case, but async hooks and even async context have their own issues if you want to use it on the backend and if any of your dependencies do not handle resource pooling correctly. |
For Angular, the "Zone problem" is being solved by Signals |
To clarify the scope of the issue, does this issue persist with Angular 17? |
@birkskyum Define which issue. Since v15, Angular forces ES2022 but uses |
@JeanMeche Interesting. For those of us who are okay with zone.js not being able to patch await, is there a way to disable the babel-plugin-transform-async-to-promises entirely and ship async/await in the bundle to the browser? |
What you're looking for is being tracked at angular/angular-cli#22191. It's not possible at the moment. |
@JeanMeche , it says on the ticket you linked that it didn't make it to the Roadmap due to a lack of upvotes. Do you know if it's planned, or if another vote is needed? |
This is likely something that will be worked on when zoneless will be officially supported. (Which isn't the case right now on standalone applications, |
Target in app tsconfig is set to es2022 by the Angular CLI, so we must set it as well to be consistent in the rest of the tooling. Angular compilation later uses browserslist for further transpilations. Target in unit tests is kept at es2016 because of a known bug in Angular: angular/angular#31730
Target in app tsconfig is set to es2022 by the Angular CLI, so we must set it as well to be consistent in the rest of the tooling. Angular compilation later uses browserslist for further transpilations. Target in unit tests is kept at es2016 because of a known bug in Angular: angular/angular#31730
Target in app tsconfig is set to es2022 by the Angular CLI, so we must set it as well to be consistent in the rest of the tooling. Angular compilation later uses browserslist for further transpilations. Target in unit tests is kept at es2016 because of a known bug in Angular: angular/angular#31730
I'm going to close this issue as it's inactionable for us. Zone will never be able to be dropped on a page and be compatible with For this and a variety of other reasons, Angular is working on making the zone.js dependency optional, via a "zoneless" mode of operation. Additionally, #54952 allows change detection to work with native |
Awesome that it's being worked on. If zone.js can't be patched, this ticket can still be resolved by shipping the zoneless alternative, with a clear migration path, thus making this issue actionable. This 7 year old ticket, influencing things as important as TC39 discussions, really should only be closed with the release of that new feature. If it's to stay closed anyway, without action, I'd ask if it could be done so as "not planned" rather than "completed", because many are tracking this ticket. Where will the new "zoneless" mode of operation be tracked going forward? |
Target in app tsconfig is set to es2022 by the Angular CLI, so we must set it as well to be consistent in the rest of the tooling. Angular compilation later uses browserslist for further transpilations. Target in unit tests is kept at es2016 because of a known bug in Angular: angular/angular#31730
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
this issue is similar with #715, if we use chrome v8 async/await and compile angular with tsconfig target 'ES2017', then typescript will not generate __awaiter code and use native async/await.
and the following logic will fail
unlike typescript transpiler, native async/await will first yield from test, and then call promise.then
for continuation when await something. So Zone currentFrame will become root.
The sequence of above logic when call
await delay(100)
will look likePromise.prototype.then
Based on the spec,
https://tc39.github.io/ecmascript-asyncawait/#abstract-ops-async-function-await
Step8 is not be executed immediately but in the microTask queue after the current function execution.
I checked Chrome v8 source,
https://chromium.googlesource.com/v8/v8/+/refs/heads/5.5.10/src/js/promise.js
ZoneAwarePromise is not treated as native one, so Chrome v8 enqueue a micro task to perform
then
call. This maybe the reason.And it seems the logic is totally changed in v8 6.0, so I will try the chromium 6.0 to see what happened.
The text was updated successfully, but these errors were encountered: