Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

RFC: zone-less / zone-full directives #3446

Closed
1 of 2 tasks
ywarezk opened this issue Jun 3, 2022 · 6 comments
Closed
1 of 2 tasks

RFC: zone-less / zone-full directives #3446

ywarezk opened this issue Jun 3, 2022 · 6 comments

Comments

@ywarezk
Copy link

ywarezk commented Jun 3, 2022

Information

When working in zone-less mode, a lot of libraries will not function properly.
Example of an open issue in @angular/material: angular/components#9169

if you still want to use the vast angular libraries out there, it might be more realistic to work in an hybrid zone-less mode and not completely zone-less.
In this mode you do not remove Zone.js, but you can optimize performance by setting a subtree of components to work outside of the zone.js.

For example with this directive your entire app can be zoneless but when you place angular material components you can wrap them in this directive thus enjoying zoneless performance with support for other libraries.

My suggestion is to add 2 simple directives:

  • ngrxZoneLess - gets a template and creates that template outside Zone.js
  • ngrxZoneFull - gets a template and runs it inside the angular zone.

Simplistic example of ngrxZoneLess:

@Directive({
  selector: '[ngrxZoneLess]'
})
export class ZoneLessDirective  implements AfterViewInit {

   constructor(
    private templateRef: TemplateRef<any>,
    private viewContainer: ViewContainerRef,
    private _ngzone: NgZone
  ) { }

  ngAfterViewInit() {
    this._ngzone.runOutsideAngular(() => {
      this.viewContainer.createEmbeddedView(this.templateRef);
    })
  }

}

The usage of that directive would be:

<div *ngrxZoneLess>
  <p> this entire area of my app is running in zoneless, in fact the majority of my app can run zoneless using this</p>
  <!-- this will not work cause angular material do not support zone less -->
  <mat-checkbox>
  </mat-checkbox>


  <div *ngrxZoneFull>
        <p>Although im wrapped in zone-less, this section i want to be zone aware cause im using material</p>
        <!-- this will work allowing to place the majority of my app in zone less while maintaining compatibility with other libraries -->
      <mat-checkbox >
      </mat-checkbox>
  </div>

</div>
  

The ngrxZoneFull would be similar only using NgZone.prototype.run.

I think Hybrid mode for zone-less would give you the ability to enjoy the performance gain of zone-less, while avoid troubleshooting libraries that do not support zone-less.

There is the cost the zone.js would have to be included in the bundle, but I think it is worth it.

Describe any alternatives/workarounds you're currently using

Currently I created that directive myself.

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@brandonroberts
Copy link
Member

Thanks @ywarezk. I don't fully see why this directive would exist inside @ngrx/component though instead of in userland code

I don't know of many people running zoneless applications in practice and those that do are more aware of the implications of it not working as seamlessly because it's not fully supported by the framework itself.

@ywarezk
Copy link
Author

ywarezk commented Jun 8, 2022

@brandonroberts
this is from the @ngrx/component docs (first line: https://ngrx.io/guide/component)

@ngrx/component
Component is a set of primitive reactive helpers to enable fully reactive, Zoneless applications. They give more control over rendering, and provide further reactivity for Angular applications.

In addition @ngrx/component already contains ngrxPush so it makes sense to me

@ywarezk ywarezk closed this as completed Jun 13, 2022
@ywarezk ywarezk reopened this Jun 13, 2022
@brandonroberts
Copy link
Member

@ywarezk that's correct, but I think introducing this directive could be more constraining than helpful. @markostanimirovic has been working on the component package, so I'll defer to him on whether this should be added. We'll revisit sometime after the v14 release, but will leave it open for now.

@markostanimirovic
Copy link
Member

Hi @ywarezk,

I'm not sure this would be a safe solution for partial exclusion of zone.js because declarables that behave differently in zone-full and zone-less mode (such as LetDirective, PushPipe, or declarables from other libraries) usually check if zone.js is present or not by checking the instance of NgZone service. With *ngrxZoneLess we could not provide NoopNgZone to its children, so if we use e.g. ngrxPush within the *ngrxZoneLess template, the ngrxPush will inject NgZone instance instead of NoopNgZone.

Also, it can be confusing that we can partially include zone.js in zone-less applications via *ngrxZoneFull directive. However, the *ngrxZoneFull directive would not work if the zone.js script is excluded and ngZone is set to noop in the bootstrap config.

@ywarezk
Copy link
Author

ywarezk commented Jul 2, 2022

Thanks @markostanimirovic

I do think that the ngrxPush can do the same job that it is doing by examining Zone.current.name !== 'angular' and not examining the instance of NgZone to be NoopNgZone.
In the case of examining for NoopNgZone the ngrxPush will not work properly on cases where you supply your own custom NgZone.

platformBrowserDynamic().bootstrapModule(AppModule, {
  ngZone: MyNgZone
})

as for the second point it should be well written in the docs about the fact that zoneless means that almost every library that relies on events will not work properly,
and about hybrid mode that zonejs should be included.
In addition it is easy to detect that case and throw an informative exception:

@Directive({
    ...
})
export class ZoneLessDirective {
    constructor(_ngZone: NgZone) {
        if (_ngZone instanceof NoopNgZone) {
            throw new Error('Hybrid mode requires you to include zonejs')
        }
    }
}

perhaps @brandonroberts is right and there is no room for those directives here.
In the meantime I will prepare a zoneless library with those directives, hopefully in the future they will be part of ngrx.

@markostanimirovic
Copy link
Member

In the meantime I will prepare a zoneless library with those directives, hopefully in the future they will be part of ngrx.

That would be great! You can also create additional helpers for hybrid mode such as registering event listeners outside of the NgZone (more info).

Can't wait to try out your library! 🙂

Btw, I'll convert this issue to discussion. Feel free to share your library in the comment.

@ngrx ngrx locked and limited conversation to collaborators Jul 2, 2022
@markostanimirovic markostanimirovic converted this issue into discussion #3478 Jul 2, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

3 participants