Skip to content

Commit 626e9b8

Browse files
bengrybengry
authored
Merge pull request #28 from Microsoft/render-prop-update-fix
Fix ng-templates rendered as render props not updating when internal bindings change
2 parents c994049 + 66cb2f6 commit 626e9b8

35 files changed

+456
-115
lines changed

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

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
ElementRef,
1010
Injector,
1111
Input,
12+
NgZone,
1213
OnChanges,
1314
Renderer2,
1415
SimpleChanges,
@@ -48,6 +49,27 @@ export type JsxRenderFunc<TContext> = (context: TContext) => JSX.Element;
4849
export type ContentClassValue = string[] | Set<string> | { [klass: string]: any };
4950
export type ContentStyleValue = string | StyleObject;
5051

52+
/**
53+
* Optional options to pass to `ReactWrapperComponent`.
54+
*/
55+
export interface WrapperComponentOptions {
56+
/**
57+
* Whether the host's `display` should be set to the root child node's`display`.
58+
* @default `false`.
59+
*/
60+
readonly setHostDisplay?: boolean;
61+
62+
/**
63+
* The zone to use to track changes to inner (Angular) templates & components.
64+
* @default `undefined`.
65+
*/
66+
readonly ngZone?: NgZone;
67+
}
68+
69+
const defaultWrapperComponentOptions: WrapperComponentOptions = {
70+
setHostDisplay: false,
71+
};
72+
5173
/**
5274
* Base class for Angular @Components wrapping React Components.
5375
* Simplifies some of the handling around passing down props and CSS styling on the host component.
@@ -57,6 +79,9 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
5779
private _contentClass: Many<ContentClassValue>;
5880
private _contentStyle: ContentStyleValue;
5981

82+
private _ngZone: NgZone;
83+
private _shouldSetHostDisplay: boolean;
84+
6085
protected abstract reactNodeRef: ElementRef<HTMLElement>;
6186

6287
/**
@@ -109,13 +134,16 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
109134
public readonly elementRef: ElementRef<HTMLElement>,
110135
private readonly changeDetectorRef: ChangeDetectorRef,
111136
private readonly renderer: Renderer2,
112-
private readonly setHostDisplay: boolean = false
113-
) {}
137+
{ setHostDisplay, ngZone }: WrapperComponentOptions = defaultWrapperComponentOptions
138+
) {
139+
this._ngZone = ngZone;
140+
this._shouldSetHostDisplay = setHostDisplay;
141+
}
114142

115143
ngAfterViewInit() {
116144
this._passAttributesAsProps();
117145

118-
if (this.setHostDisplay) {
146+
if (this._shouldSetHostDisplay) {
119147
this._setHostDisplay();
120148
}
121149

@@ -164,8 +192,12 @@ export abstract class ReactWrapperComponent<TProps extends {}> implements AfterV
164192
return undefined;
165193
}
166194

195+
if (!this._ngZone) {
196+
throw new Error('To create an input JSX renderer you must pass an NgZone to the constructor.');
197+
}
198+
167199
if (input instanceof TemplateRef) {
168-
const templateRenderer = createTemplateRenderer(input, additionalProps);
200+
const templateRenderer = createTemplateRenderer(input, this._ngZone, additionalProps);
169201
return (context: TContext) => templateRenderer.render(context);
170202
}
171203

‎libs/core/src/lib/declarations/omit.d.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { KnownKeys } from './known-keys';
5-
4+
/**
5+
* Inverse of `Pick<T, K>`.
6+
*/
67
export type Omit<T, K extends keyof T> = Pick<T, Exclude<KnownKeys<T> & keyof T, K>>;

‎libs/core/src/lib/declarations/string-map.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
/**
5+
* An object with a `string`-based index signature.
6+
*/
47
export type StringMap<T = any> = { [index: string]: T };

‎libs/core/src/lib/renderer/react-content.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ export interface ReactContentProps {
2323
/**
2424
* Creates a new `ReactContent` element.
2525
* @param children The children to append to the `ReactContent` element.
26-
* @param additionalProps _Optional_. @see `ReactContentProps`
27-
* @returns
26+
* @param additionalProps _Optional_. @see `ReactContentProps`.
2827
*/
2928
export function createReactContentElement(children: ReadonlyArray<HTMLElement>, additionalProps?: ReactContentProps) {
3029
return React.createElement(ReactContent, {
@@ -36,12 +35,12 @@ export function createReactContentElement(children: ReadonlyArray<HTMLElement>,
3635
/**
3736
* @internal
3837
*/
39-
export interface InternalReactContentProps extends ReactContentProps {
38+
interface InternalReactContentProps extends ReactContentProps {
4039
readonly [CHILDREN_TO_APPEND_PROP]: ReadonlyArray<HTMLElement>;
4140
}
4241

4342
/**
44-
* Render any `HTMLElement`s (including Angular components) as a child of React components.
43+
* Render any `HTMLElement`s as a child of React components.
4544
* Supports two rendering modes:
4645
* 1. `legacy` - append `<react-content>` as the root, and nest the `children-to-append` underneath it.
4746
* 2. `new` (**default**) - append the `children-to-append` to the parent of this component, and hide the `<react-content>` element.

‎libs/core/src/lib/renderer/react-template.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { EmbeddedViewRef, NgZone, TemplateRef } from '@angular/core';
5+
import * as React from 'react';
6+
import * as ReactDOM from 'react-dom';
7+
import { Subscription } from 'rxjs';
8+
import { throttleTime } from 'rxjs/operators';
9+
10+
const DEBUG = false;
11+
const TEMPLATE_DETECT_CHANGES_THROTTLE_MS = 250;
12+
13+
/**
14+
* @internal
15+
*/
16+
export interface ReactTemplateProps {
17+
/**
18+
* Experimental rendering mode.
19+
* Uses a similar approach to `router-outlet`, where the child elements are added to the parent, instead of this node, and this is hidden.
20+
* @default false
21+
*/
22+
legacyRenderMode?: boolean;
23+
}
24+
25+
/**
26+
* Creates a new `ReactTemplate` element.
27+
* @param templateRef The template to render.
28+
* @param context The context to inject the template.
29+
* @param ngZone A zone used for tracking changes in the template.
30+
* @param additionalProps _Optional_. @see `ReactTemplateProps`.
31+
*/
32+
export function createReactTemplateElement<TContext extends object | void>(
33+
templateRef: TemplateRef<TContext>,
34+
context: TContext,
35+
ngZone: NgZone,
36+
additionalProps?: ReactTemplateProps
37+
) {
38+
return React.createElement(ReactTemplate, { ngZone, templateRef, context, ...additionalProps });
39+
}
40+
41+
/**
42+
* @internal
43+
*/
44+
interface InternalReactTemplateProps<TContext extends object | void> extends ReactTemplateProps {
45+
ngZone: NgZone;
46+
templateRef: TemplateRef<TContext>;
47+
context: TContext;
48+
}
49+
50+
/**
51+
* Render an `ng-template` as a child of a React component.
52+
* Supports two rendering modes:
53+
* 1. `legacy` - append `<react-content>` as the root, and nest the `children-to-append` underneath it.
54+
* 2. `new` (**default**) - append the `children-to-append` to the parent of this component, and hide the `<react-content>` element.
55+
* (similar to how `router-outlet` behaves in Angular).
56+
*/
57+
export class ReactTemplate<TContext extends object | void> extends React.Component<
58+
InternalReactTemplateProps<TContext>
59+
> {
60+
private _embeddedViewRef: EmbeddedViewRef<TContext>;
61+
private _ngZoneSubscription: Subscription;
62+
63+
componentDidUpdate() {
64+
// Context has changes, trigger change detection after pushing the new context in
65+
Object.assign(this._embeddedViewRef.context, this.props.context);
66+
this._embeddedViewRef.detectChanges();
67+
}
68+
69+
componentDidMount() {
70+
const { context, ngZone, templateRef } = this.props;
71+
72+
this._embeddedViewRef = templateRef.createEmbeddedView(context);
73+
const element = ReactDOM.findDOMNode(this);
74+
if (DEBUG) {
75+
console.warn('ReactTemplate Component > componentDidMount > childrenToAppend:', {
76+
rootNodes: this._embeddedViewRef.rootNodes,
77+
});
78+
}
79+
80+
const hostElement = this.props.legacyRenderMode ? element : element.parentElement;
81+
82+
this._embeddedViewRef.rootNodes.forEach(child => hostElement.appendChild(child));
83+
84+
// Detect the first cycle's changes, and then subscribe for subsequent ones.
85+
this._embeddedViewRef.detectChanges();
86+
87+
// Throttling the detect changes to an empirically selected value so we don't overload too much work.
88+
// TODO: This needs some better solution to listen to changes to the binding sources of the template.
89+
this._ngZoneSubscription = ngZone.onUnstable
90+
.pipe(throttleTime(TEMPLATE_DETECT_CHANGES_THROTTLE_MS))
91+
.subscribe(() => {
92+
this._embeddedViewRef.markForCheck();
93+
});
94+
}
95+
96+
componentWillUnmount() {
97+
this._ngZoneSubscription.unsubscribe();
98+
99+
if (this._embeddedViewRef) {
100+
this._embeddedViewRef.destroy();
101+
}
102+
}
103+
104+
render() {
105+
return React.createElement('react-template', !this.props.legacyRenderMode && { style: { display: 'none' } });
106+
}
107+
}

‎libs/core/src/lib/renderer/renderprop-helpers.ts

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { ComponentRef, EmbeddedViewRef, TemplateRef } from '@angular/core';
4+
import { ComponentRef, NgZone, TemplateRef } from '@angular/core';
55
import { createReactContentElement, ReactContentProps } from '../renderer/react-content';
6+
import { createReactTemplateElement } from './react-template';
67

78
export interface RenderPropContext<TContext extends object> {
89
readonly render: (context: TContext) => JSX.Element;
@@ -16,28 +17,16 @@ function renderReactContent(rootNodes: HTMLElement[], additionalProps?: ReactCon
1617
* Wrap a `TemplateRef` with a `JSX.Element`.
1718
*
1819
* @param templateRef The template to wrap
20+
* @param ngZone A zone used for tracking & triggering updates to the template
1921
* @param additionalProps optional additional props to pass to the `ReactContent` object that will render the content.
2022
*/
2123
export function createTemplateRenderer<TContext extends object>(
2224
templateRef: TemplateRef<TContext>,
25+
ngZone: NgZone,
2326
additionalProps?: ReactContentProps
2427
): RenderPropContext<TContext> {
25-
let viewRef: EmbeddedViewRef<TContext> | null = null;
26-
let renderedJsx: JSX.Element | null = null;
27-
2828
return {
29-
render: (context: TContext) => {
30-
if (!viewRef) {
31-
viewRef = templateRef.createEmbeddedView(context);
32-
renderedJsx = renderReactContent(viewRef.rootNodes, additionalProps);
33-
} else {
34-
// Mutate the template's context
35-
Object.assign(viewRef.context, context);
36-
}
37-
viewRef.detectChanges();
38-
39-
return renderedJsx;
40-
},
29+
render: (context: TContext) => createReactTemplateElement(templateRef, context, ngZone, additionalProps),
4130
};
4231
}
4332

‎libs/core/src/public-api.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@ export * from './lib/components/wrapper-component';
66
export * from './lib/declarations/public-api';
77
export * from './lib/renderer/components/Disguise';
88
export { getPassProps, passProp, PassProp } from './lib/renderer/pass-prop-decorator';
9-
export { ReactContent } from './lib/renderer/react-content';
9+
export { createReactContentElement, ReactContent, ReactContentProps } from './lib/renderer/react-content';
10+
export * from './lib/renderer/react-template';
1011
export { registerElement } from './lib/renderer/registry';

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
// Licensed under the MIT License.
33

44
import { InputRendererOptions, JsxRenderFunc, ReactWrapperComponent } from '@angular-react/core';
5-
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Input, OnInit, Renderer2, ViewChild } from '@angular/core';
5+
import {
6+
ChangeDetectionStrategy,
7+
ChangeDetectorRef,
8+
Component,
9+
ElementRef,
10+
Input,
11+
OnInit,
12+
Renderer2,
13+
ViewChild,
14+
} from '@angular/core';
615
import { IBreadcrumbItem, IBreadcrumbProps } from 'office-ui-fabric-react/lib/Breadcrumb';
716

817
@Component({
@@ -22,8 +31,7 @@ import { IBreadcrumbItem, IBreadcrumbProps } from 'office-ui-fabric-react/lib/Br
2231
[styles]="styles"
2332
[theme]="theme"
2433
[RenderItem]="renderItem && onRenderItem"
25-
[ReduceData]="onReduceData"
26-
>
34+
[ReduceData]="onReduceData">
2735
</Breadcrumb>
2836
`,
2937
styles: ['react-renderer'],

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, Renderer2, ViewChild } from '@angular/core';
4+
import {
5+
ChangeDetectionStrategy,
6+
ChangeDetectorRef,
7+
Component,
8+
ElementRef,
9+
NgZone,
10+
Renderer2,
11+
ViewChild,
12+
} from '@angular/core';
513
import { FabBaseButtonComponent } from './base-button.component';
614

715
@Component({
@@ -57,7 +65,7 @@ export class FabActionButtonComponent extends FabBaseButtonComponent {
5765
@ViewChild('reactNode')
5866
reactNodeRef: ElementRef;
5967

60-
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
61-
super(elementRef, changeDetectorRef, renderer);
68+
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2, ngZone: NgZone) {
69+
super(elementRef, changeDetectorRef, renderer, ngZone);
6270
}
6371
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33

44
import { InputRendererOptions, JsxRenderFunc, ReactWrapperComponent } from '@angular-react/core';
5-
import { ChangeDetectorRef, ElementRef, EventEmitter, Input, OnInit, Output, Renderer2 } from '@angular/core';
5+
import { ChangeDetectorRef, ElementRef, EventEmitter, Input, NgZone, OnInit, Output, Renderer2 } from '@angular/core';
66
import { IButtonProps } from 'office-ui-fabric-react/lib/Button';
77

88
export abstract class FabBaseButtonComponent extends ReactWrapperComponent<IButtonProps> implements OnInit {
@@ -90,8 +90,8 @@ export abstract class FabBaseButtonComponent extends ReactWrapperComponent<IButt
9090
onRenderChildren: (props?: IButtonProps, defaultRender?: JsxRenderFunc<IButtonProps>) => JSX.Element;
9191
onRenderMenuIcon: (props?: IButtonProps, defaultRender?: JsxRenderFunc<IButtonProps>) => JSX.Element;
9292

93-
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
94-
super(elementRef, changeDetectorRef, renderer, true);
93+
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2, ngZone: NgZone) {
94+
super(elementRef, changeDetectorRef, renderer, { ngZone, setHostDisplay: true });
9595

9696
// coming from React context - we need to bind to this so we can access the Angular Component properties
9797
this.onMenuClickHandler = this.onMenuClickHandler.bind(this);

0 commit comments

Comments
 (0)