refactor(common): update NgTemplateOutlet to no longer rely on context swapping (#51887)

devversion · alxhub · commit 7426948ff63a · 2023-10-04T08:14:35.000-07:00
The context of an embedded view ref at some point was switched from a getter to an actual assignable property. This is something we reverted with the previous commit as it introduces additional complexity for our generated code (in terms of closures capturing the `ctx`). This change impacted the template outlet code because we actively relied on swapping out the full context if the user changes it. Previousl, before we allowed to swap out the context (in v16), we mutated the initial view context if it didn't change structurally- and in other cases the view was re-created. We improved this performance aspect with the changes to allow for the context to be swapped out + actually also fixed a bug where the initial context object was mutated and the user could observe this change. This commit adjusts for context not being replacable- while still keeping the bugs fixed and preserving the performance wins of not having to destroy/re-create the view whenever the context changes. Benchmarks: https://hackmd.io/J0Ci_JzxQ0K1AA1omXhIQQ PR Close #51887
diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json
@@ -19,4 +19,4 @@
       "dark-theme": 34598
     }
   }
-}
+}
diff --git a/packages/common/src/directives/ng_template_outlet.ts b/packages/common/src/directives/ng_template_outlet.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, EmbeddedViewRef, Injector, Input, OnChanges, SimpleChanges, TemplateRef, ViewContainerRef} from '@angular/core';
+import {Directive, EmbeddedViewRef, Injector, Input, OnChanges, SimpleChange, SimpleChanges, TemplateRef, ViewContainerRef} from '@angular/core';
 
 /**
  * @ngModule CommonModule
@@ -57,30 +57,57 @@ export class NgTemplateOutlet<C = unknown> implements OnChanges {
 
   constructor(private _viewContainerRef: ViewContainerRef) {}
 
-  /** @nodoc */
   ngOnChanges(changes: SimpleChanges) {
-    if (changes['ngTemplateOutlet'] || changes['ngTemplateOutletInjector']) {
+    if (this._shouldRecreateView(changes)) {
       const viewContainerRef = this._viewContainerRef;
 
       if (this._viewRef) {
         viewContainerRef.remove(viewContainerRef.indexOf(this._viewRef));
       }
 
-      if (this.ngTemplateOutlet) {
-        const {
-          ngTemplateOutlet: template,
-          ngTemplateOutletContext: context,
-          ngTemplateOutletInjector: injector,
-        } = this;
-        this._viewRef =
-            viewContainerRef.createEmbeddedView(
-                template, context, injector ? {injector} : undefined) as EmbeddedViewRef<C>;
-      } else {
+      // If there is no outlet, clear the destroyed view ref.
+      if (!this.ngTemplateOutlet) {
         this._viewRef = null;
+        return;
       }
-    } else if (
-        this._viewRef && changes['ngTemplateOutletContext'] && this.ngTemplateOutletContext) {
-      this._viewRef.context = this.ngTemplateOutletContext;
+
+      // Create a context forward `Proxy` that will always bind to the user-specified context,
+      // without having to destroy and re-create views whenever the context changes.
+      const viewContext = this._createContextForwardProxy();
+      this._viewRef = viewContainerRef.createEmbeddedView(this.ngTemplateOutlet, viewContext, {
+        injector: this.ngTemplateOutletInjector ?? undefined,
+      });
     }
   }
+
+  /**
+   * We need to re-create existing embedded view if either is true:
+   * - the outlet changed.
+   * - the injector changed.
+   */
+  private _shouldRecreateView(changes: SimpleChanges): boolean {
+    return !!changes['ngTemplateOutlet'] || !!changes['ngTemplateOutletInjector'];
+  }
+
+  /**
+   * For a given outlet instance, we create a proxy object that delegates
+   * to the user-specified context. This allows changing, or swapping out
+   * the context object completely without having to destroy/re-create the view.
+   */
+  private _createContextForwardProxy(): C {
+    return <C>new Proxy({}, {
+      set: (_target, prop, newValue) => {
+        if (!this.ngTemplateOutletContext) {
+          return false;
+        }
+        return Reflect.set(this.ngTemplateOutletContext, prop, newValue);
+      },
+      get: (_target, prop, receiver) => {
+        if (!this.ngTemplateOutletContext) {
+          return undefined;
+        }
+        return Reflect.get(this.ngTemplateOutletContext, prop, receiver);
+      },
+    });
+  }
 }
diff --git a/packages/common/test/directives/ng_template_outlet_spec.ts b/packages/common/test/directives/ng_template_outlet_spec.ts
@@ -318,6 +318,30 @@ describe('NgTemplateOutlet', () => {
 
     expect(fixture.nativeElement.textContent).toBe('Hello World');
   });
+
+  it('should properly bind context if context is unset initially', () => {
+    @Component({
+      imports: [NgTemplateOutlet],
+      template: `
+        <ng-template #tpl let-name>Name:{{name}}</ng-template>
+        <ng-template [ngTemplateOutlet]="tpl" [ngTemplateOutletContext]="ctx"></ng-template>
+      `,
+      standalone: true,
+    })
+    class TestComponent {
+      ctx: {$implicit: string}|undefined = undefined;
+    }
+
+    const fixture = TestBed.createComponent(TestComponent);
+    fixture.detectChanges();
+
+    expect(fixture.nativeElement.textContent).toBe('Name:');
+
+    fixture.componentInstance.ctx = {$implicit: 'Angular'};
+    fixture.detectChanges();
+
+    expect(fixture.nativeElement.textContent).toBe('Name:Angular');
+  });
 });
 
 const templateToken = new InjectionToken<string>('templateToken');

Original file line number	Diff line number	Diff line change
`@@ -19,4 +19,4 @@`
`19`	`19`	`"dark-theme": 34598`
`20`	`20`	`}`
`21`	`21`	`}`
`22`		`-}`
	`22`	`+}`