X Tutup
The Wayback Machine - https://web.archive.org/web/20210219180102/https://github.com/angular/angular/issues/31730
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

Open
JiaLiPassion opened this issue Apr 16, 2017 · 95 comments · May be fixed by angular/zone.js#795 or angular/zone.js#1140
Open
Assignees
Milestone

Comments

@JiaLiPassion
Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Apr 16, 2017

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
Member

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

@JiaLiPassion JiaLiPassion commented Apr 19, 2017

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented May 21, 2017

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
Member

@mhevery mhevery commented May 21, 2017

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented May 21, 2017

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

JiaLiPassion referenced this issue in JiaLiPassion/zone.js May 28, 2017
@amcdnl
Copy link
Contributor

@amcdnl amcdnl commented May 31, 2017

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Jun 1, 2017

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

@shellmann shellmann commented Jul 16, 2018

@JiaLiPassion can you say something about the current status?

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Jul 16, 2018

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Jul 17, 2018

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

@royalrover
Copy link

@royalrover royalrover commented Jul 18, 2018

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Jul 18, 2018

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

@alamothe
Copy link

@alamothe alamothe commented Jul 29, 2018

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

@JiaLiPassion JiaLiPassion commented Aug 24, 2018

@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

@JiaLiPassion JiaLiPassion commented Sep 9, 2018

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Oct 15, 2018

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

@JiaLiPassion JiaLiPassion commented Oct 15, 2018

@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

@dfahlander dfahlander commented Oct 15, 2018

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

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Oct 15, 2018

@dfahlander, thanks for the clarification!

@rvalimaki
Copy link

@rvalimaki rvalimaki commented Mar 27, 2019

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

@JiaLiPassion JiaLiPassion commented Mar 27, 2019

@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 Eugeny commented Mar 27, 2019

@JiaLiPassion thanks for clarifying

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Dec 16, 2020

The async_hooks version of zone.js is still developing, will update the status when I have some progress.

@Rob4226
Copy link

@Rob4226 Rob4226 commented Jan 27, 2021

Suppose one is willing to manually run ChangeDetectorRef detectChanges or Application.tick in any async/await blocks in order to move past ES2016. Are there any other problems that would need to be addressed besides change detection not running automatically when using async/await?

@atamansv
Copy link

@atamansv atamansv commented Jan 29, 2021

The ice has broken angular/angular-cli@e2e8d57

@rezonant
Copy link

@rezonant rezonant commented Jan 29, 2021

That's great news, but I do hope the team will keep exploring ways to enable this outside of angular. I use zonejs outside of angular for many purposes

@ajbouh
Copy link

@ajbouh ajbouh commented Jan 29, 2021

That's great news, but I do hope the team will keep exploring ways to enable this outside of angular. I use zonejs outside of angular for many purposes

I strongly agree.

One option to consider is using ahead of time compilation to decorate all async operations with needed state preservation idioms. This approach would assume of course that you aren't doing any runtime loading of uninstrumented code.

@rezonant
Copy link

@rezonant rezonant commented Jan 29, 2021

Yeah maybe, personally I'm thinking the best bang for our buck is petitioning the Typescript developers to allow for downleveled async/await on higher ES targets. Might be an uphill battle since Zone.js users are probably one of the only groups who really need it :-\

@vivainio
Copy link

@vivainio vivainio commented Jan 30, 2021

@rezonant best bang for buck is probably moving Angular users away from Zone. It's not really pulling its weight anymore, and keeps us stuck in the past

@mlc-mlapis
Copy link
Contributor

@mlc-mlapis mlc-mlapis commented Jan 30, 2021

The zone concept still has significant potential. The problem is that it has been expected its native implementation in browsers spec.

@rezonant
Copy link

@rezonant rezonant commented Jan 30, 2021

You are always free to use a different change detection strategy other than Zone.js. Personally for Angular I use (and prefer) Zone.js by default and use Zone escape (via NgZone#runOutsideAngular()) for high performance timers and optimizations. It seems like a lot of people suggest removing Zone.js from their Angular project without communicating the development costs that will be associated with that change: Either committing to reactive state management or manually triggering change detection like in the Angular.js days. These are high cost alternatives for applications which often will not see tangible benefits from removing the overhead of Zone.js.

It's fine if you as a developer want to do that, but it's never as easy as "just remove Zone.js" :-)

More importantly, and separately from the debate about state management, Zone.js solves use cases that are impossible, leaky or difficult without it, which have little or nothing to do with Angular:

  • It can be used (outside Angular) in testing frameworks to track test case execution without the need for done(), avoiding false positives where a done() call, await, or promise return is missed. This is how the Razmin testing framework handles asynchronous tests (disclosure: it's my library).
  • It can be used to attribute log messages to execution contexts (ie, stamping the UUID of an HTTP request onto a log, or noting which background task started the thread of execution)
  • It is used (fully transparently) in SSR to detect when it is safe and correct to send the HTML back to the browser, because all pending asynchronous tasks have been completed. Note that this has nothing to do with your choice of change detection strategy.
  • It is used to intercept global APIs in specific contexts, even while other code outside the context uses the original/unaffected global APIs
  • It is used to know when a transaction can be automatically closed

These are just the use cases I myself have used it for over the last few years. There are countless more that have yet to be thought of. Zone.js is a fantastic library, and I can only hope that it can make the leap to supporting async/await so we can enjoy it until the ES committee can be persuaded to adopt support for something like it into the language and the runtime.

All this is not to say that the library itself does not have rough edges, or that there aren't paths for improvements.

  • The Zone API is complicated and difficult to understand
  • Zones (even with zone-aware stack traces) can hamper debuggability quite a bit
  • Zones do have a performance impact, though I think a lot of people think the performance impact they see in Angular is because of Zone, but in reality Zone is just telling Angular when something has completed, it is then Angular which triggers a change detection. This change detection is where a lot of the observed overhead comes from
  • And of course, Zones cannot patch native async/await, the whole reason we're here.
@pauldraper
Copy link

@pauldraper pauldraper commented Jan 31, 2021

@rezonant those are some noble ideas, but Zone.js is all but dead. It doesn't even work with Node.js async/await, despite async_hooks being available.

The Zone.js API is overwrought and over complicated.

Its carcass was merged into the Angular repo from its original home because no one else had any independent interest in it.

@ajbouh
Copy link

@ajbouh ajbouh commented Jan 31, 2021

@rezonant those are some noble ideas, but Zone.js is all but dead. It doesn't even work with Node.js async/await, despite async_hooks being available.

The Zone.js API is overwrought and over complicated.

Its carcass was merged into the Angular repo from its original home because no one else had any interest in it.

@pauldraper any chance you can point to a specific bug or failing test case?

@pauldraper
Copy link

@pauldraper pauldraper commented Jan 31, 2021

You mean that Node.js native async/await is not supported?

angular/zone.js#795

#32796

@ajbouh
Copy link

@ajbouh ajbouh commented Jan 31, 2021

Yes, thank you.

I am using zone.js because it's a necessary dependency of opentelemetry in the browser. Unfortunate that such a new observability project is so dependent on a fragile and aging approach.

@rezonant
Copy link

@rezonant rezonant commented Jan 31, 2021

The Zone.js API is overwrought and over complicated.

I agree, that's why I started modelling an alternative API which solves the same problem-space in a simpler way. Check out https://github.com/rezonant/rezone . The API is much simpler, and might be a more approachable path to standardization. The prototype implementation is built on top of Zone.js for simplicity. However, the design of Zone.js' API does not invalidate the underlying concepts; we still need a way to observe the lifecycle of chains of asynchronous operations at runtime.

Its carcass was merged into the Angular repo from its original home because no one else had any independent interest in it.

@pauldraper I think this is a bit harsh. You might consider the developers who originally built it and all the challenges they had to solve, and how much value the entire community has extracted from it, even though it seems to be a punching bag amongst the under-informed in the community. Myself and others are posting here because we not only see the potential of what Zones in Javascript can do, but that we are already making use of it in production scenarios today (outside of Angular).

Honestly it's not surprising that it took years for anyone to realize how to apply Zone.js to other use cases. The concepts themselves are subtle, and as we've both pointed out, the library itself (and certainly its documentation) is rough, and the API design is, yes, overcomplicated and formidable. One need only search for "Zone Angular" to see the reams of "What exactly is Zone.js" and "How exactly does Zone.js work" articles which themselves are a bit hand wavey and overly focused on the use case of change detection.

Yes, thank you.

I am using zone.js because it's a necessary dependency of opentelemetry in the browser. Unfortunate that such a new observability project is so dependent on a fragile and aging approach.

Though I'm not familar with OpenTelemetry's codebase, I'm familiar with what it is meant to do, and I have no doubt it is using Zone.js specifically for the type of benefits I mention above- it is able to track execution contexts across asynchronous calls. As far as I'm aware, there is no other way to do that other than hotpatching the APIs like Zone.js does.

It's not that the approach is aging, but simply that the final design of ES' async/await did not allow for interception as needed for a user-land observability framework like Zone.js.


Trying to veer back on to the task at hand, let me sum up the possibilities for how to solve native async/await in Zone.js generally:

  1. Handle it at compile-time like Angular is now, but in a way that can be enjoyed by non-Angular users like OpenTelemetry, Razmin and Alterior
    • While this isn't ideal, it seems like the only practical option in the short to medium term.
    • Such a solution would probably be best implemented as both a Babel plugin and a Typescript plugin for the best developer ergonomics
    • It would have to be used in the build process by the end developer to ensure that all native async/await usages (across packages) are instrumented
    • Ideally would instrument, not down-level -- If we're going to go through the work to implement it, we might as well take advantage of the reduced overhead and platform integration that runtime-supported async/await offers.
  2. Petition the TS developers for a reprieve: The ability to down-level async/await while targetting higher ES versions. I think this will be difficult to accomplish, and does nothing to solve the issue for non-TS users
    • Furthermore, TS does not modify code used by packages anyway (except possibly when bundling?). When consuming a package that targets ES2017+, it won't matter that you target ES2016 because the library's async/await usages remain native, thus causing a Zone leak when that code is executed.
  3. Champion a proposal to ES to introduce an observability API (similar to async_hooks) that could be made available in all ES runtimes.
  4. Champion a proposal to bring a Zone or Execution-Context primitive into ES itself. This has already been attempted by Domenic Denicola prior but was unsuccessful. My rezone repository attempts to reimagine that effort

For anyone new to the issue: Support for native async/await can't be trivially implemented because await does not trigger the creation of Promise like the down-leveled awaiter-style system used by Typescript and Babel does. Because no instance of Promise is created, Zone.js' own ZoneAwarePromise cannot feed back task information into the current zone, thus any execution context which is resuming after the native await will fall outside of the Zone they were originally created in. To put it simply, there is no place for Zone to "hook".

Node.js' async_hooks (not supported in the browser, nor Deno, nor other ES runtimes) is part of the solution, but the implementation work has been lagging behind for quite some time.

EDIT: Edited to add note that TS downleveling can't solve for libraries

@pauldraper
Copy link

@pauldraper pauldraper commented Feb 1, 2021

Looking to TS for a solution is flawed. Besides being unworkable for libraries, it also breaks the ability to use Angular from JS.

Otherwise, I agree. IIRC there was an early proposal for async_hooks-like standard for ES or browsers, but I can't remember what it was.

My criticisms is of Zone.js, not context tracing in general. (Complain all you want about there not being standard runtime support available, but Zone.js doesn't even use mechanisms for Node.js that were available...it was effectively abandoned long ago, perhaps because of its flawed design, perhaps other reasons.)

I look with great interest towards the success of your project, @rezonant .

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Feb 1, 2021

I will try to support zone.js with nodejs async_hooks so it will be more efficient in NodeJS environment, but async hooks is a little different with the current design of zone.js, for now zone.js support interceptable hooks, you can totally override async task behavior, but async hooks is just a notification, you can do something such as trace/log/monitoring, but you can not change the default async tasks' behavior. So maybe in nodejs there will be 2 modes for zone.js.

@rezonant
Copy link

@rezonant rezonant commented Feb 1, 2021

Ah of course- that does explain the difficulty-- wrapping the task handler to detect exceptions would be needed for Razmin and probably OpenTelemetry (assuming it has exception tracing), but for Angular it might be enough. I wonder though: is async_hooks enough to allow introspecting the zone heirarchy? If not, that means zone-locals would be out as well. If the current zone can be ascertained then perhaps async_hooks + smart global exception handling would do the trick to support onError- and ironically Zone.js API can handle this better than rezone API, which fundamentally needs the ability to modify task callbacks. At least with Zone's setup the way the onError hook is executed is abstracted out more.

I'm curious if there are other known use cases for modifying task callbacks (other than catching exceptions)?

EDIT: After a bit more thought, it must be possible to establish the Zone heirarchy, assuming before()/after() happen within the same turn as the task handler (documentation doesn't seem to be clear on that)

@rezonant
Copy link

@rezonant rezonant commented Feb 1, 2021

Wait, isn't this a bit of a problem?

Another subtlety with promises is that before and after callbacks are run only on chained promises. That means promises not created by then()/catch() will not have the before and after callbacks fired on them. For more details see the details of the V8 PromiseHooks API.
https://nodejs.org/api/async_hooks.html#async_hooks_before_asyncid

@avatsaev
Copy link
Contributor

@avatsaev avatsaev commented Feb 8, 2021

zonejs was a mistake.

@rezonant
Copy link

@rezonant rezonant commented Feb 9, 2021

@avatsaev:
Why so negative? Zone.js support for async/await in ES2017+ within Angular itself is effectively solved as of the commit referenced above. You're free to use it or opt for some other way to manage change detection using ChangeDetectionStrategy.

Though it's arguably not the best place to debate the merits of Zone.js, given that we have a lot of work before this issue can be fully closed, I'd welcome your thoughts on why you feel that way, provided Jia Li or any other Angular team members / moderators don't object, as after all, this is their venue.

@vladimiry
Copy link

@vladimiry vladimiry commented Feb 9, 2021

Why so negative?

There is a lot of why, here is just a short list:

  • It's complex, adds weight, requires to be maintained.
  • It's a monkey patching. So by definition not a forward-looking idea. It might potentially make the patched functions work differently in comparison with native/original behavior and also might affect the performance. Async/await is/was the first blocker, there might be more in the future.
  • There is no such thing as an interface put on top of implementation. So if you started using it from day one there won't be a simple way to replace the change detection by changing the interface implementation without a need to rethink and rewrite a huge part of the code base. No interface and so no simple/static/type-safe way to detect all the places where the change detection is being triggered. So you got Angular built on DI and other enterprise-grade fancy ideas but here you left with just that "implicit magic".
  • It's enabled by default and so most of the devs likely just go with it without a doubt.
  • It hides how things work from those who just go with defaults.

Not saying that Angular itself is a bad thing and of course not saying that zone.js is completely useless. But if I would be forced to start a project on Angular, these days I'd go with a reactive store, for example using @ngrx/store + @ngrx/component-store, onPush change detection, and fully disabled zone.js thing.

@rezonant
Copy link

@rezonant rezonant commented Feb 9, 2021

I'm going to approach your post as an Angular (frontend) developer. As I've noted above, Zone.js is far more useful outside of Angular than in, but it's clear you are coming at it from that angle, and there's plenty to discuss.

Before I get into a detailed response, I'll note that I've heard all this before, many times. What I haven't heard is folks with a deep understanding of Zone.js and the real down-to-the-metal performance impacts of Angular as a framework weigh in too often. I hope my two cents is useful as someone who has built heavily performance intensive apps; I have built several realtime audio/video processing apps for the broadcast industry built with Angular. I'm not saying that any of what you've said is necessarily incorrect, just that it feels misguided to me, and doesn't fit my experience of the technology stack based on the projects I've worked on.

It's complex

This word is doing a lot here. Let me address it this way: As an Angular app developer, when it comes to Zone.js and how its used in Angular, I only care about the following things:

  1. I care when Zone.js is not performant
    When this happens, one should escape from Zone.js using NgZone#runOutsideAngular(). That is what it's for. This is relevant is when you are doing highly performance sensitive work such as processing audio frames, updating meters, rendering canvases, or other requestAnimationFrame style use cases. Escape from the zone, and return using NgZone#run(). Don't ignore the tools the Angular team has given you. What I've found is that in these scenarios the real problem is Angular's change detection itself. When you are solving those problems, you sometimes have to omit using Angular's model binding system entirely to accomplish the goal. In that context, using Zone.js to handle change detection for a settings dialog is not the bottleneck.

  2. I care when Zone.js impacts debuggability
    Zone.js definitely impacts debuggability. Stack traces are difficult. But not unworkably so. Throwing the baby out with the bathwater for this point seems woefully short sighted.

  3. I care when my code escapes from its Zone
    When Zone is not adequately handling the available async-capable APIs, this happens. More on this below.

adds weight

For change detection, you are correct. But this isn't going to be relevant when you are doing the typical Angular app- it will be relevant when you are doing requestAnimationFrame(), tight timers, or other code which frequently waits for promises to complete. When this happens, do the above: Escape from the zone, and return to do change detection. Don't rearchitect your entire application simply to serve your 1% use case- most of your app is forms over data, and Zone.js' change detection strategy works for that. The escape is there for the 1% where it doesn't.

requires to be maintained.

Maintained as new async web APIs are added yes. Not maintained by the app developer of course, but maintained by the Zone.js developers themselves. So then the question becomes, how often are new async opportunities added to Web APIs? Well, you can look at the commits in https://github.com/angular/angular/commits/master/packages/zone.js where Jia Li has been maintaining Zone.js, and really there hasn't been much needed to keep it working (with the async/await-pocalypse the main exception). Seems like more work has gone into Bazel and testing which was going to be done regardless of the merits of Zone.js as a library.

It's a monkey patching. So by definition not a forward-looking idea.

Monkey patching is adding new user-land functionality to existing interfaces. Zone.js doesn't do this, it instruments those interfaces. I would agree if Zone added new methods to String, or modified the parameters that Regexp took. But this isn't the case.

It might potentially make the patched functions work differently in comparison with native/original behavior

This is possible, and I think we'd all like to see Zones as a language primitive where it has no chance of this happening, but the runtime timing model of Javascript is such that there ultimately are only 3 critical concepts: The macrotask, the microtask, and the turn. This is how every Javascript runtime you've ever used is constructed, and Zone.js only needs to layer on instrumentation for recording when these concepts are scheduled and when they complete. Thus, Zone.js need only express the scheduling and macro/micro task behavior of the API it is patching. It's complex when you look under the hood, but the runtime semantics of setTimeout don't change with every newly published ES release, so the chance of breaking semantics is rather low for a given API once the patching is stable. Would love to hear about concrete cases where you've seen Zone.js modify runtime semantics (other than stack traces 😅) in a way that impacted your business logic.

and also might affect the performance.

Yes, but as I've noted in previous posts and have hinted at above, it is easy to equate the cost of Angular's change detection cycle with Zone.js itself. Remember, all Zone.js does is let Angular know when your callback is done executing, it is the actual change detection process which is expensive.
If you don't believe me that Zone.js isn't dramatically impacting your frontend performance, just try this: Take your app that uses reactive stores with onPush and add Zone.js back in in your main.ts, just as an experiment:

import 'zone.js';
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';

import { AppModule } from './app/app.module';
import { environment } from './environments/environment';

let zone = Zone.current.fork();

zone.run(() => {
    platformBrowserDynamic().bootstrapModule(AppModule)
    .catch(err => console.error(err));
})

Technically speaking, running Angular in a Zone isn't even required for Zone to be active. Just loading it will cause all the patches to be in place. Nonetheless, I'm quite certain that almost all of the perceived performance issues you are referring to will be gone, because the expensive part is change detection, not Zone.js itself. And this is why disabling change detection with NgZone#runOutsideAngular is so useful for managing performance overhead, not because it eliminates the overhead of Zone.js, but because it eliminates the overhead of extra change detection cycles.

Async/await is/was the first blocker, there might be more in the future.

I'll grant you that it is possible a future feature could cause a problem, but that's not a reason to ignore the use cases that only Zone.js or a similar context tracing / observability solution enables.

There is no such thing as an interface put on top of implementation. So if you started using it from day one there won't be a simple way to replace the change detection by changing the interface implementation without a need to rethink and rewrite a huge part of the code base. No interface and so no simple/static/type-safe way to detect all the places where the change detection is being triggered. So you got Angular built on DI and other enterprise-grade fancy ideas but here you left with just that "implicit magic".

This is only a major concern if you are thinking that you will need to remove Zone.js for either (A) performance reasons (see above) or (B) compatibility problems causing zone escape, of which (other than bugs found and fixed in Zone.js along the way) there has really only been one existential problem: async/await. In the case of bugs with Zone.js that have existed before which may have allowed your callbacks to modify Angular's model state without a corresponding change detection cycle, you always had the option of re-entering the zone in that specific circumstance to rectify the issue. So far it has only been async/await which has not had an easy straightforward fix.

It's enabled by default and so most of the devs likely just go with it without a doubt.

It's the default because most apps are forms over data. The "just work" part of it has always been Angular's promise, and Zone.js' use in Angular 2+ was just the next obvious step. If you remember back in the Angular.js days, you might remember that the built-in HTTP service would trigger $digest automatically after your callback handled the HTTP response. Zone.js was just the next evolution of this, providing an "it just works" experience in the apps which Angular was originally designed to create.

It hides how things work from those who just go with defaults.

This is what a framework is for. Angular also hides how efficient virtual DOM diffing works, and how the dependency injection container is properly constructed from your disparate sets of DI providers. That doesn't make it an inherently bad thing.

Not saying that Angular itself is a bad thing and of course not saying that zone.js is completely useless. But if I would be forced to start a project on Angular, these days I'd go with a reactive store, for example using @ngrx/store + @ngrx/component-store, onPush change detection, and fully disabled zone.js thing.

Do your thing, but I personally am not a fan of using reactive stores or ngrx. Service with a Subject is more than enough when used properly, and is a more composable model than dictating a central data store for the entire application. The complexity introduced by reactive store models is far from free and time travel debugging isn't enough for me to overhaul the entire application architecture.

@vladimiry
Copy link

@vladimiry vladimiry commented Feb 9, 2021

@rezonant, the information you provided I think might help someone to fight the temper of zone.js. I was not really saying that zone.js is not manageable but that it's, in my opinion, not a forward-looking idea and that it's hard to get rid of it. I just tend to agree with the above message that it was a mistake to introduce the zone.js thing but of course it depends on a lot of things (team, budget, kind of project, etc). It would be fine though, in my opinion, to use it for prototype-like projects (so should be disabled by default).

Service with a Subject is more than enough when used properly, and is a more composable model than dictating a central data store for the entire application

Not necessarily central, it's not dictated but flexible. It might be mixed or just component scoped store, sort of setState thing in react.

Service with a Subject is more than enough when used properly

Ngrx uses subjects internally, behavior-like one I think. The point here is unification, so you jump between projects in a productive manner since there is no need to first dive into the fancy custom built on subjects store management library. Ngrx, btw, was named as an example.

Do your thing

Thank you. But that was a way to go if I'm forced to start something new on Angular which is not the case.

@muuvmuuv
Copy link

@muuvmuuv muuvmuuv commented Feb 10, 2021

So just my noobish cents here: will there be a fix for that in zone.js? I am using Ionic with Capacitor and the Capacitor team is suggesting ES2017 for plugins, so it might be beneficial to update the app to that too. But the message in the console is annoying and having no change detection is also not a good way ^^.

I did never get behind the zone.js thing because looking at other Frameworks change detection works pretty neat without it (Svelte, Vue, preact, etc.), so I would like to hear more about this "removing zone.js and replacing it with X". Any tutorials or deeper information/docs I can read about the pro/cons and alternatives? I am looking forward for a simpler more integrated CD in Angular :).

@rezonant
Copy link

@rezonant rezonant commented Feb 10, 2021

I believe Angular 11.2 will have support for ES2017+ when it's released if all goes well, so you can start targeting that ES level with that version and still use zonejs. If you want to opt out of automatic change detection, then you need to handle it via some other means or do it manually. Look up ChangeDetectionStrategy. The options are to use a reactive store approach like ngrx or to manually trigger change detection when you need the view to update. If you already have a sizable app using zonejs, as myself and others have pointed out, it may be expensive to retrofit a reactive store into your app. If you are already on Angular 11 then it's probably best to adopt 11.2 when it comes out.

@vladimiry
Copy link

@vladimiry vladimiry commented Feb 10, 2021

I believe Angular 11.2 will have support for ES2017+

ES2017+ TS target but excluding native async/await since this part will be internally transpiled (TS doesn't support partial switches but just "target" so custom transpilling needed)?

@rezonant
Copy link

@rezonant rezonant commented Feb 10, 2021

Yes it down levels async/await using babel internally in the angular compiler, so it should be transparent to the app developer. If you are not using angular CLI then you wouldn't benefit from the chanhe, but then again you always could have put the babel plugin into your custom webpack configuration, so that could be set up in a project today if desired.

The only downsides on that is perhaps less efficiency (since native async/await can benefit from runtime optimizations) but that was already the case on ES2016 target and at least you'll be able to use newer ES features in your codebase, and libraries using native async/await will be automatically downlevelled as well, so no worries about using libraries that make use of that feature, whereas before it was a subtle foot gun, since you couldn't control whether a lib used native async/await without a compiler pass.

It's a nice stopgap to ensure devs can continue to target newer ES versions, and this issue is here to track progress around a more wholistic solution that doesn't need to downlevel async/await while still ensuring code cannot unintentionally escape from a zone.

@rezonant
Copy link

@rezonant rezonant commented Feb 11, 2021

It would be fine though, in my opinion, to use it for prototype-like projects (so should be disabled by default).

I think it should be the default, but I wouldn't be too upset if it were made not to be -- I'd hope Angular continues to offer it as an option regardless, but even if they don't, it is actually quite trivial to implement Zone based change detection without Angular's help so I would probably add it back in and publish it as an addon package for those who want to use it :-)

Service with a Subject is more than enough when used properly

Ngrx uses subjects internally, behavior-like one I think. The point here is unification, so you jump between projects in a productive manner since there is no need to first dive into the fancy custom built on subjects store management library. Ngrx, btw, was named as an example.

So to be clear, I'm not talking about building a store based on Subject. I'm talking about building services which expose Observables. "Service with a Subject" is just the popular vernacular for the approach, which stands in contrast to the recent uptick in interest in React style data management strategies like Redux, Flux, Ngrx, or other reactive stores. "Service with a Subject" isn't really a replacement for a reactive store itself, it is just a way of organizing your app's data flow, which almost any Angular developer is probably already familiar with, even if they didn't know that there was a term for it.

The canonical example is a UserService that tracks the current user:

@Injectable()
export class UserService {
    private _userChanged = new BehaviorSubject<User>(null);
    get userChanged() {
        return this._userChanged;
    }

    // ....
}

A component interested in the current user would subscribe:

@Component()
export class LoginStatusComponent implements OnInit, OnDestroy {
    constructor(
        private userService : UserService
    ) { 
    }

    private subsink = new SubSink();
    user : User;

    ngOnInit() {
        this.subsink.add(this.userService.userChanged.subscribe(user => this.user = user));
    }

    ngOnDestroy() {
        this.subsink.unsubscribe();
    }
}

I imagine there are a great many of these UserService (or AuthService, AccountService, LoginService, whatever you decide to call it) within the Angular apps out in production right now, using this sort of model.

But you can use this for everything- even for "instance" style subscriptions. Say you have a ChatRoom class that represents an incoming stream of chat messages in a particular "room"? ChatService#getRoom(roomID) could return a ChatRoom class instance which has a messageReceived observable, which you then subscribe to.

This really doesn't have anything to do with change detection, but simply data flow in your application. And indeed, reactive store solutions also don't have anything to do with change detection, except that you can hook into when the state has been definitively updated, and only trigger change detection when that happens.

The point here is unification

I think you mean "conformity" :-)

Do your thing

Thank you. But that was a way to go if I'm forced to start something new on Angular which is not the case.

I guess the difference is I proudly start new apps in Angular to this day. If you would prefer starting an app in a different framework that's great, there's nothing wrong with that, but I prefer Angular over the alternatives (though I do enjoy learning about those frameworks and the new concepts arising from them, and I certainly don't think Angular is the perfect framework).

Nonetheless, thank you @vladimiry for the lively and civil debate about the pros/cons on this topic, and thanks for all subscribers to this issue for entertaining the discussion :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
X Tutup