Merge pull request #4 from Microsoft/bugfix/inner-react-render-props-not-render

React-wrapped components inside inside ng-template don't re-render on changes

Author: bengry

apps/demo/src/app/counter/counter.component.ts

@@ -1,10 +1,9 @@
-import { Component, OnInit, ChangeDetectionStrategy, ChangeDetectorRef } from '@angular/core';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component } from '@angular/core';
 
 @Component({
   selector: 'counter',
   template: `
-    <div>{{ count }}</div>
-    <button (click)="onClick()">+</button>
+    <fab-default-button [text]="count + '+'" (onClick)="onClick()"></fab-default-button>
     `,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })

libs/core/package.json

@@ -1,7 +1,7 @@
 {
   "$schema": "../../node_modules/ng-packagr/package.schema.json",
   "name": "@angular-react/core",
-  "version": "0.3.1",
+  "version": "0.4.0",
   "ngPackage": {
     "deleteDestPath": true,
     "whitelistedNonPeerDependencies": [

libs/core/src/lib/components/wrapper-component.ts

@@ -1,20 +1,10 @@
-import {
-  AfterViewInit,
-  ChangeDetectorRef,
-  ComponentFactoryResolver,
-  ComponentRef,
-  ElementRef,
-  Injector,
-  Input,
-  OnChanges,
-  SimpleChanges,
-  TemplateRef,
-  Type,
-} from '@angular/core';
+import { AfterViewInit, ChangeDetectorRef, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, Input, OnChanges, Renderer2, SimpleChanges, TemplateRef, Type } from '@angular/core';
 import toStyle from 'css-to-style';
 import { ReactContentProps } from '../renderer/react-content';
 import { isReactNode } from '../renderer/react-node';
+import { isReactRendererData } from '../renderer/renderer';
 import { renderComponent, renderFunc, renderTemplate } from '../renderer/renderprop-helpers';
+import { afterRenderFinished } from '../utils/render/render-delay';
 import { unreachable } from '../utils/types/unreachable';
 
 // Forbidden attributes are still ignored, since they may be set from the wrapper components themselves (forbidden is only applied for users of the wrapper components)
@@ -93,6 +83,7 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
   constructor(
     public readonly elementRef: ElementRef,
     private readonly changeDetectorRef: ChangeDetectorRef,
+    private readonly renderer: Renderer2,
     private readonly setHostDisplay: boolean = false
   ) {}
 
@@ -102,6 +93,19 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
     if (this.setHostDisplay) {
       this._setHostDisplay();
     }
+
+    // NOTE: Workaround/fix for Issue #5 (https://github.com/Microsoft/angular-react/issues/5).
+    // The wrapper component isn't added to the root react nodes list when it's inside a `ReactContent` node, we manually add it (note that the root nodes list is a `Set`, so it won't duplicate nodes if already exist).
+    // There's potentially a better solution instead of this
+    const rendererData = this.renderer.data;
+    if (isReactRendererData(rendererData)) {
+      afterRenderFinished(() => {
+        const nativeElement = this.reactNodeRef.nativeElement;
+        if (isReactNode(nativeElement)) {
+          rendererData.addRootNode(nativeElement);
+        }
+      });
+    }
   }
 
   ngOnChanges(changes: SimpleChanges) {
@@ -165,10 +169,14 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
    */
   protected createRenderPropHandler<TProps extends object>(
     renderInputValue: InputRendererOptions<TProps>,
-    jsxRenderer?: JsxRenderFunc<TProps>,
-    additionalProps?: ReactContentProps
+    options?: {
+      jsxRenderer?: JsxRenderFunc<TProps>;
+      additionalProps?: ReactContentProps;
+    }
   ): (props?: TProps, defaultRender?: JsxRenderFunc<TProps>) => JSX.Element | null {
-    const renderer = jsxRenderer || this.createInputJsxRenderer(renderInputValue, additionalProps);
+    const renderer =
+      (options && options.jsxRenderer) ||
+      this.createInputJsxRenderer(renderInputValue, options && options.additionalProps);
 
     return (props?: TProps, defaultRender?: JsxRenderFunc<TProps>) => {
       if (!renderInputValue) {

libs/core/src/lib/renderer/renderer.ts

@@ -16,7 +16,7 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
   // React elements cannot be "inserted" and later have their props
   // updated, so the "insert", or React.Render, can only be done once the
   // element has been fully defined.  Only the topmost [root] nodes are added here.
-  public reactRootNodes: Array<ReactNode> = [];
+  public reactRootNodes: Set<ReactNode> = new Set();
 
   private isRenderPending: boolean;
   // This flag can only be set to true from outside.  It can only be reset
@@ -56,29 +56,39 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
     // are ReadOnly.
 
     // Workaround for ReactNodes inside ReactContent being added to the root of the VDOM and not removed from the VDOM when unmounted from the DOM.
-    for (let i = 0; i < this.reactRootNodes.length; i++) {
-      const node = this.reactRootNodes[i];
+    this.reactRootNodes.forEach(node => {
       if (
         !isReactNode(node.parent) &&
         !document.body.contains(node.parent) &&
         ReactDOM.unmountComponentAtNode(node.parent)
       ) {
-        this.reactRootNodes.splice(i--, 1);
+        this.reactRootNodes.delete(node);
       }
-    }
+    });
 
     if (this.isRenderPending) {
       // Remove root nodes that are pending destroy after render.
-      this.reactRootNodes = this.reactRootNodes.filter(node => !node.render().destroyPending);
+      this.reactRootNodes = new Set(Array.from(this.reactRootNodes).filter(node => !node.render().destroyPending));
       this.isRenderPending = false;
     }
   }
 }
 
-class ReactRenderer implements Renderer2 {
-  readonly data: StringMap<any> = Object.create(null);
+export const isReactRendererData = (data: StringMap): data is ReactRendererData =>
+  data && typeof (data as ReactRendererData).addRootNode === 'function';
+
+export interface ReactRendererData {
+  readonly addRootNode: (node: ReactNode) => void;
+}
+
+export class ReactRenderer implements Renderer2 {
+  readonly data: ReactRendererData = {
+    addRootNode: (node: ReactNode) => {
+      this.rootRenderer.reactRootNodes.add(node);
+    },
+  };
 
-  constructor(private rootRenderer: AngularReactRendererFactory) {}
+  constructor(public readonly rootRenderer: AngularReactRendererFactory) {}
 
   destroy(): void {}
 
@@ -128,7 +138,8 @@ class ReactRenderer implements Renderer2 {
         console.warn('Renderer > appendChild > asDOM > parent:', parent.toString(), 'node:', node.toString());
       }
       node.setRenderPendingCallback = this.rootRenderer.setRenderPendingCallback;
-      this.rootRenderer.reactRootNodes.push(node);
+
+      this.rootRenderer.reactRootNodes.add(node);
       node.parent = parent;
       return;
     }

libs/core/src/lib/utils/render/render-delay.ts

@@ -0,0 +1,3 @@
+export const afterRenderFinished = (callback: Function) => {
+  setTimeout(callback, 0);
+};

libs/fabric/package.json

@@ -1,7 +1,7 @@
 {
   "$schema": "../../node_modules/ng-packagr/package.schema.json",
   "name": "@angular-react/fabric",
-  "version": "0.3.1",
+  "version": "0.4.0",
   "ngPackage": {
     "lib": {
       "entryFile": "public-api.ts",
@@ -70,7 +70,7 @@
   ],
   "private": false,
   "peerDependencies": {
-    "@angular-react/core": "^0.3.0",
+    "@angular-react/core": "^0.4.0",
     "@angular/common": "^6.1.0",
     "@angular/core": "^6.1.0",
     "@angular/platform-browser-dynamic": "^6.1.0",

libs/fabric/src/lib/components/breadcrumb/breadcrumb.component.ts

@@ -1,13 +1,5 @@
 import { InputRendererOptions, JsxRenderFunc, ReactWrapperComponent } from '@angular-react/core';
-import {
-  ChangeDetectionStrategy,
-  Component,
-  ElementRef,
-  Input,
-  OnInit,
-  ViewChild,
-  ChangeDetectorRef,
-} from '@angular/core';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Input, OnInit, Renderer2, ViewChild } from '@angular/core';
 import { IBreadcrumbItem, IBreadcrumbProps } from 'office-ui-fabric-react/lib/Breadcrumb';
 
 @Component({
@@ -66,8 +58,8 @@ export class FabBreadcrumbComponent extends ReactWrapperComponent<IBreadcrumbPro
 
   onRenderItem: (props?: IBreadcrumbItem, defaultRender?: JsxRenderFunc<IBreadcrumbItem>) => JSX.Element;
 
-  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef) {
-    super(elementRef, changeDetectorRef);
+  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
+    super(elementRef, changeDetectorRef, renderer);
   }
 
   ngOnInit() {

libs/fabric/src/lib/components/button/action-button.component.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, ViewChild } from '@angular/core';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Renderer2, ViewChild } from '@angular/core';
 import { FabBaseButtonComponent } from './base-button.component';
 
 @Component({
@@ -54,7 +54,7 @@ export class FabActionButtonComponent extends FabBaseButtonComponent {
   @ViewChild('reactNode')
   reactNodeRef: ElementRef;
 
-  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef) {
-    super(elementRef, changeDetectorRef);
+  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
+    super(elementRef, changeDetectorRef, renderer);
   }
 }

libs/fabric/src/lib/components/button/base-button.component.ts

@@ -1,5 +1,5 @@
 import { InputRendererOptions, JsxRenderFunc, ReactWrapperComponent } from '@angular-react/core';
-import { ChangeDetectorRef, ElementRef, EventEmitter, Input, OnInit, Output } from '@angular/core';
+import { ChangeDetectorRef, ElementRef, EventEmitter, Input, OnInit, Output, Renderer2 } from '@angular/core';
 import { IButtonProps } from 'office-ui-fabric-react/lib/Button';
 
 export abstract class FabBaseButtonComponent extends ReactWrapperComponent<IButtonProps> implements OnInit {
@@ -87,8 +87,8 @@ export abstract class FabBaseButtonComponent extends ReactWrapperComponent<IButt
   onRenderChildren: (props?: IButtonProps, defaultRender?: JsxRenderFunc<IButtonProps>) => JSX.Element;
   onRenderMenuIcon: (props?: IButtonProps, defaultRender?: JsxRenderFunc<IButtonProps>) => JSX.Element;
 
-  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef) {
-    super(elementRef, changeDetectorRef, true);
+  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
+    super(elementRef, changeDetectorRef, renderer, true);
 
     // coming from React context - we need to bind to this so we can access the Angular Component properties
     this.onMenuClickHandler = this.onMenuClickHandler.bind(this);

libs/fabric/src/lib/components/button/command-bar-button.component.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, ViewChild } from '@angular/core';
+import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Renderer2, ViewChild } from '@angular/core';
 import { FabBaseButtonComponent } from './base-button.component';
 
 @Component({
@@ -54,7 +54,7 @@ export class FabCommandBarButtonComponent extends FabBaseButtonComponent {
   @ViewChild('reactNode')
   reactNodeRef: ElementRef;
 
-  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef) {
-    super(elementRef, changeDetectorRef);
+  constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
+    super(elementRef, changeDetectorRef, renderer);
   }
 }