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

angular with tsconfig target ES2017 async/await will not work with zone.js #31730

Closed
JiaLiPassion opened this issue Apr 16, 2017 · 154 comments · May be fixed by angular/zone.js#795 or angular/zone.js#1140
Closed
Assignees
Labels
area: zones feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq4: critical
Milestone

Comments

@JiaLiPassion
Copy link
Contributor

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

(click) = test();
const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
const test = async () => {
      console.log(Zone.current.name) // will output 'angular'
      await delay(100);
      console.log(Zone.current.name) // will output 'root'
    }

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 like

  1. delay function return a ZoneAwarePromise, Zone.current is angular
  2. test function return native Promise which generate from chrome v8 by await, and test() execution is finished.
  3. So Zone.currentZone is transite from angular->root
  4. ZoneAwarePromise which generated from step1 was chained by called by Promise.prototype.then
  5. delay timeout is executed, and ZoneAwarePromise resolved
  6. the chained Promise is resolved , but the Zone.current is root.

Based on the spec,
https://tc39.github.io/ecmascript-asyncawait/#abstract-ops-async-function-await


1. Let asyncContext be the running execution context.
2. Let promiseCapability be ! NewPromiseCapability(%Promise%).
3. Let resolveResult be ! Call(promiseCapability.[[Resolve]], undefined, « value »).
4. Let onFulfilled be a new built-in function object as defined in AsyncFunction Awaited Fulfilled.
5. Let onRejected be a new built-in function object as defined in AsyncFunction Awaited Rejected.
6. Set onFulfilled and onRejected's [[AsyncContext]] internal slots to asyncContext.
7. Let throwawayCapability be NewPromiseCapability(%Promise%).
8. Perform ! PerformPromiseThen(promiseCapability.[[Promise]], onFulfilled, onRejected, throwawayCapability).
9. Remove asyncContext from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
10. Set the code evaluation state of asyncContext such that when evaluation is resumed with a Completion resumptionValue the following steps will be performed:
11. Return resumptionValue.
12. Return.

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

function ResolvePromise(promise, resolution) {
  if (resolution === promise) {
    return RejectPromise(promise,
                         %make_type_error(kPromiseCyclic, resolution),
                         true);
  }
  if (IS_RECEIVER(resolution)) {
    // 25.4.1.3.2 steps 8-12
    try {
      var then = resolution.then;
    } catch (e) {
      return RejectPromise(promise, e, true);
    }
    // Resolution is a native promise and if it's already resolved or
    // rejected, shortcircuit the resolution procedure by directly
    // reusing the value from the promise.
    if (IsPromise(resolution) && then === PromiseThen) {
      var thenableState = GET_PRIVATE(resolution, promiseStateSymbol);
      if (thenableState === kFulfilled) {
        // This goes inside the if-else to save one symbol lookup in
        // the slow path.
        var thenableValue = GET_PRIVATE(resolution, promiseResultSymbol);
        FulfillPromise(promise, kFulfilled, thenableValue,
                       promiseFulfillReactionsSymbol);
        SET_PRIVATE(promise, promiseHasHandlerSymbol, true);
        return;
      } else if (thenableState === kRejected) {
        var thenableValue = GET_PRIVATE(resolution, promiseResultSymbol);
        if (!HAS_DEFINED_PRIVATE(resolution, promiseHasHandlerSymbol)) {
          // Promise has already been rejected, but had no handler.
          // Revoke previously triggered reject event.
          %PromiseRevokeReject(resolution);
        }
        // Don't cause a debug event as this case is forwarding a rejection
        RejectPromise(promise, thenableValue, false);
        SET_PRIVATE(resolution, promiseHasHandlerSymbol, true);
        return;
      }
    }
    if (IS_CALLABLE(then)) {
      // PromiseResolveThenableJob
      var id;
      var name = "PromiseResolveThenableJob";
      var instrumenting = DEBUG_IS_ACTIVE;
      %EnqueueMicrotask(function() {
        if (instrumenting) {
          %DebugAsyncTaskEvent({ type: "willHandle", id: id, name: name });
        }
        // These resolving functions simply forward the exception, so
        // don't create a new debugEvent.
        var callbacks = CreateResolvingFunctions(promise, false);
        try {
          %_Call(then, resolution, callbacks.resolve, callbacks.reject);
        } catch (e) {
          %_Call(callbacks.reject, UNDEFINED, e);
        }
        if (instrumenting) {
          %DebugAsyncTaskEvent({ type: "didHandle", id: id, name: name });
        }
      });
      if (instrumenting) {
        id = ++lastMicrotaskId;
        %DebugAsyncTaskEvent({ type: "enqueue", id: id, name: name });
      }
      return;
    }
  }
  FulfillPromise(promise, kFulfilled, resolution, promiseFulfillReactionsSymbol);
}

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.

@mhevery
Copy link
Contributor

mhevery commented Apr 18, 2017

/FYI @domenic

The short answer is that it is not possible to intercept native promises in the VM. (Native promises is used by async/await) This is a known issue.

We have two approaches to this problem:

  1. Try to standardize on Zone as part of the browser https://github.com/domenic/zones
  2. Add hooks to the browser VM which would allow one to intercept native promises https://docs.google.com/document/d/1rda3yKGHimKIhg5YeoAmCOtyURgsbTH_qaYR79FELlk

Out of the two approaches I think second one (setPromiseHook) is more likely.

This effort has been a bit on back-burner, but I will restart the discussions with @domenic.

@JiaLiPassion
Copy link
Contributor Author

@mhevery , got it, I will also do some research about the v8 hooks. thank you!

@JiaLiPassion
Copy link
Contributor Author

node.js begin to support async_hooks,

https://github.com/nodejs/node-eps/blob/master/006-asynchooks-api.md
nodejs/node#13000
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 , do you think this is the correct direction? please review.

@mhevery
Copy link
Contributor

mhevery commented May 21, 2017 via email

@JiaLiPassion
Copy link
Contributor Author

@mhevery , got it, I will try to use async-hook to implement Zone in node.js and let you review after finish!

@amcdnl
Copy link
Contributor

amcdnl commented May 31, 2017

@JiaLiPassion - Awesome stuff!
@mhevery - any idea when this could get merged up?

@JiaLiPassion
Copy link
Contributor Author

@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.

@amcdnl
Copy link
Contributor

amcdnl commented Jun 1, 2017

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.

@enko
Copy link

enko commented May 9, 2018

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.

@shellmann
Copy link

@JiaLiPassion can you say something about the current status?

@JiaLiPassion
Copy link
Contributor Author

@enko, @shellmann, this issue is still pending because there is no proper way to patch async/await promise in Javascript world. In nodejs, it is possible to do the patch and I have made some POC to verify it. But in browser, it is still impossible to do that, so we will wait whether zone will pass TC39 proposal or browser can open some PromiseHook like API to us.

So now, sorry this issue can not be resolved, so Angular can not work with ES2017 currently.

@enko
Copy link

enko commented Jul 17, 2018

@JiaLiPassion Would that be https://github.com/domenic/zones?

@JiaLiPassion
Copy link
Contributor Author

@enko, yes, it is, it has been in stage 0 for about two years...

@royalrover
Copy link

@JiaLiPassion How about node.js now? Can we resolve the question with async_hook? has examples? thx!

@JiaLiPassion
Copy link
Contributor Author

@royalrover, yes, with async_hooks, this issue can be resolved, I am working on it in this PR #795.

@alamothe
Copy link

Just want to point out the "obvious" workaround for Node.js, which is to downgrade target to ES2015 😄 . Thanks for your great work!

@rvalimaki
Copy link

rvalimaki commented Aug 21, 2018

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!

@mhevery

@dfahlander
Copy link

dfahlander commented Aug 24, 2018

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.

@JiaLiPassion
Copy link
Contributor Author

@dfahlander, thank you for the post, I will definitely read it, and I will contact you if I have any questions.

@JiaLiPassion
Copy link
Contributor Author

@dfahlander, thanks for your post again, I have sent you an email, please check it.

@JiaLiPassion
Copy link
Contributor Author

@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.

async function test1() {
  console.log('before', Zone.current.name);
  await asyncFunction();
  console.log('after', Zone.current.name); // here is still not working
}

function test2() {
  test1();
}

Zone.current.fork({name: 'test'}).run(test2);

In this case, test2 itself is not an async function, and inside it, it will call an async function, in this case, zone.js doesn’t know when await of test1 begin, so zone.js can not keep the zone before await.

I think Dexie doesn't handle this too, because https://github.com/dfahlander/Dexie.js/blob/e956de3b74cf3ac5f1ed2fa2b97b53e4fe60a3e1/src/classes/dexie/transaction-helpers.ts#L57 the code here also require scopeFunc is an AsyncFunction, but if the scopeFunc is not async, and inside scopeFunc there is await call, it can't be traced.

Not sure my understanding is correct, please confirm, thanks!

@dfahlander
Copy link

dfahlander commented Oct 15, 2018

That is true. Maybe the check (scopeFunc.constructor === AsyncFunction) might not be the best solution. Thinking of whether it would be better to always do incrementExpectedAwaits() and inspect whether returnValue is NativePromise, and if not, call decrementExpectedAwaits() directly....

@JiaLiPassion
Copy link
Contributor Author

@dfahlander, thanks for the response, in this case,

async function test1() {
  console.log('before', Zone.current.name);
  await asyncFunction();
  console.log('after', Zone.current.name); // here is still not working
}

function test2() {
  test1();
}

Zone.current.fork({name: 'test'}).run(test2);

the return value of function test2 is void, so there is no way to check it is a NativePromise or not.
So we still need some PromiseHook to make all cases work!

@dfahlander
Copy link

True. Unless accompanied with a coding guideline of how to utilize zones together with native async await.

@JiaLiPassion
Copy link
Contributor Author

@dfahlander, thanks for the clarification!

@rvalimaki
Copy link

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?

@Eugeny
Copy link

Eugeny commented Mar 27, 2019

Excuse my possibly stupid question, but why can't zone.js override the global Promise constructor to monkey-patch the then and catch methods instead of waiting for the native hooks?

@JiaLiPassion
Copy link
Contributor Author

@rvalimaki, this is a very difficult issue to resolve, I am still trying to find out a walk around.
@Eugeny, current zone.js already monkey-patch Promise, but async/await is different, it does not use javascript Promise, it always use Native Promise.

@Eugeny
Copy link

Eugeny commented Mar 27, 2019

@JiaLiPassion thanks for clarifying

tantaman added a commit to vlcn-io/model that referenced this issue Sep 19, 2022
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.
@segevfiner
Copy link

segevfiner commented Dec 11, 2022

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 await can be wrapped with a function which will propagate the zone?

@intellild
Copy link

intellild commented Apr 24, 2023

Would it work if transform the code with babel plugin ?
Something like:

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'
  });
};

@wh1t3cAt1k
Copy link

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.

@crysislinux
Copy link

@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.

see nodejs/node#38017 (comment)

@vivainio
Copy link

vivainio commented Oct 6, 2023

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.

For Angular, the "Zone problem" is being solved by Signals

@birkskyum
Copy link

birkskyum commented Oct 17, 2023

To clarify the scope of the issue, does this issue persist with Angular 17?

@JeanMeche
Copy link
Member

@birkskyum Define which issue.

Since v15, Angular forces ES2022 but uses babel-plugin-transform-async-to-promises to convert async/await to promises. (zone.js cannot patch await). This is still true even when using the noop-zone cf angular/angular-cli#22191

@birkskyum
Copy link

birkskyum commented Oct 17, 2023

@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?

@JeanMeche
Copy link
Member

What you're looking for is being tracked at angular/angular-cli#22191. It's not possible at the moment.

@birkskyum
Copy link

birkskyum commented Oct 17, 2023

@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?

@JeanMeche
Copy link
Member

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, ɵNoopNgZone is private for that reason)

NoelDeMartin added a commit to NoelDeMartin/moodleapp that referenced this issue Nov 29, 2023
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
crazyserver pushed a commit to crazyserver/moodleapp that referenced this issue Nov 29, 2023
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
NoelDeMartin added a commit to NoelDeMartin/moodleapp that referenced this issue Nov 29, 2023
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
@alxhub
Copy link
Member

alxhub commented Mar 29, 2024

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 async/await as the API is fundamentally not patchable. Any integration must depend on compiler transforms, and our CLI uses the technique of downleveling to Promises today.

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 async/await as long as Angular is notified about the change independently (e.g. markForCheck or signals).

@alxhub alxhub closed this as completed Mar 29, 2024
@birkskyum
Copy link

birkskyum commented Mar 29, 2024

Angular is working on making the zone.js dependency optional

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?

jerico-castillo pushed a commit to jerico-castillo/my-moodleapp that referenced this issue Apr 2, 2024
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
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.