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

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

Merged
merged 5 commits into from
Aug 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions apps/demo/src/app/counter/counter.component.ts
Original file line number Diff line number Diff line change
@@ -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,
})
Expand Down
2 changes: 1 addition & 1 deletion libs/core/package.json
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down
40 changes: 24 additions & 16 deletions libs/core/src/lib/components/wrapper-component.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
) {}

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
31 changes: 21 additions & 10 deletions libs/core/src/lib/renderer/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {}

Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions libs/core/src/lib/utils/render/render-delay.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const afterRenderFinished = (callback: Function) => {
setTimeout(callback, 0);
};
4 changes: 2 additions & 2 deletions libs/fabric/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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",
Expand Down
14 changes: 3 additions & 11 deletions libs/fabric/src/lib/components/breadcrumb/breadcrumb.component.ts
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabCompoundButtonComponent 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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabDefaultButtonComponent 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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabIconButtonComponent 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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabMessageBarButtonComponent 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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabPrimaryButtonComponent 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);
}
}
Original file line number Diff line number Diff line change
@@ -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({
Expand Down Expand Up @@ -54,7 +54,7 @@ export class FabSplitButtonComponent 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);
}
}
15 changes: 3 additions & 12 deletions libs/fabric/src/lib/components/callout/callout.component.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
import { ReactWrapperComponent } from '@angular-react/core';
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ElementRef,
EventEmitter,
Input,
Output,
ViewChild,
} from '@angular/core';
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, ElementRef, EventEmitter, Input, Output, Renderer2, ViewChild } from '@angular/core';
import { ICalloutProps } from 'office-ui-fabric-react/lib/Callout';
import { ICalloutPositionedInfo } from 'office-ui-fabric-react/lib/utilities/positioning/positioning.types';

Expand Down Expand Up @@ -126,7 +117,7 @@ export class FabCalloutComponent extends ReactWrapperComponent<ICalloutProps> {
@Output()
readonly onScroll = new EventEmitter<void>();

constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef) {
super(elementRef, changeDetectorRef);
constructor(elementRef: ElementRef, changeDetectorRef: ChangeDetectorRef, renderer: Renderer2) {
super(elementRef, changeDetectorRef, renderer);
}
}
Loading