Skip to content

Commit 89d39b9

Browse files
author
Ben Grynhaus
committed
Workaround for React not removing dead nodes from VDOM
1 parent 0affdbc commit 89d39b9

File tree

3 files changed

+139
-43
lines changed

3 files changed

+139
-43
lines changed

‎.vscode/settings.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"Packagr",
77
"Renderable",
88
"Selectable",
9+
"VDOM",
910
"borderless",
1011
"checkmark",
1112
"commandbar",

‎libs/core/src/renderer/react-node.ts‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as ReactDOM from 'react-dom';
55

66
import removeUndefinedProperties from '../utils/object/remove-undefined-properties';
77
import { CHILDREN_TO_APPEND_PROP } from './react-content';
8-
import { getComponentClass } from "./registry";
8+
import { getComponentClass } from './registry';
99

1010
const DEBUG = false;
1111

@@ -37,6 +37,10 @@ export class ReactNode {
3737
this.setRenderPending();
3838
}
3939

40+
get parent(): HTMLElement | ReactNode {
41+
return this._parent;
42+
}
43+
4044
private isNotRenderable: boolean;
4145
get shouldRender() {
4246
return !this.isNotRenderable;
@@ -80,13 +84,15 @@ export class ReactNode {
8084
// If type is still a string, then no React Element matches this string.
8185
this.typeIsReactElementClass = typeof this.type !== 'string';
8286

83-
if (DEBUG) { console.log('ReactNode > tryResolveTypeIsReactElementClass > type:', this.typeName); }
87+
if (DEBUG) {
88+
console.log('ReactNode > tryResolveTypeIsReactElementClass > type:', this.typeName);
89+
}
8490
}
8591
}
8692

8793
setAttribute(name: string, value: any) {
8894
this.setAttributes({
89-
[name]: value
95+
[name]: value,
9096
});
9197
}
9298

@@ -96,7 +102,7 @@ export class ReactNode {
96102

97103
setProperty(name: string, value: any) {
98104
this.setProperties({
99-
[name]: value
105+
[name]: value,
100106
});
101107
}
102108

@@ -156,13 +162,17 @@ export class ReactNode {
156162
// parent.
157163
if (!isReactNode(this._parent)) {
158164
if (this.isDestroyPending && this._parent) {
159-
if (DEBUG) { console.log('ReactNode > render > destroy > node:', this.toString(), 'parent:', this.parent); }
165+
if (DEBUG) {
166+
console.log('ReactNode > render > destroy > node:', this.toString(), 'parent:', this.parent);
167+
}
160168
ReactDOM.unmountComponentAtNode(this._parent);
161169
return this;
162170
}
163171

164172
if (this.isRenderPending) {
165-
if (DEBUG) { console.log('ReactNode > render > node:', this.toString(), 'parent:', this.parent); }
173+
if (DEBUG) {
174+
console.log('ReactNode > render > node:', this.toString(), 'parent:', this.parent);
175+
}
166176
// It is expected that the element will be recreated and re-rendered with each attribute change.
167177
// See: https://reactjs.org/docs/rendering-elements.html
168178
ReactDOM.render(this.renderRecursive() as any, this._parent);
@@ -174,10 +184,7 @@ export class ReactNode {
174184
}
175185

176186
private renderRecursive(key?: string): React.ReactElement<{}> | string {
177-
const children =
178-
this.children
179-
? this.children.map((child, index) => child.renderRecursive(index.toString()))
180-
: [];
187+
const children = this.children ? this.children.map((child, index) => child.renderRecursive(index.toString())) : [];
181188

182189
if (this.text) {
183190
return this.text;
@@ -187,11 +194,11 @@ export class ReactNode {
187194

188195
if (key) this.props['key'] = key;
189196

190-
const clearedProps = this.transformProps(
191-
removeUndefinedProperties(this.props)
192-
);
197+
const clearedProps = this.transformProps(removeUndefinedProperties(this.props));
193198

194-
if (DEBUG) { console.warn('ReactNode > renderRecursive > type:', this.toString(), 'props:', this.props, 'children:', children); }
199+
if (DEBUG) {
200+
console.warn('ReactNode > renderRecursive > type:', this.toString(), 'props:', this.props, 'children:', children);
201+
}
195202
return React.createElement(this.type, clearedProps, children.length > 0 ? children : undefined);
196203
}
197204

@@ -220,7 +227,14 @@ export class ReactNode {
220227

221228
// This is called by Angular core when projected content is being added.
222229
appendChild(projectedContent: HTMLElement) {
223-
if (DEBUG) { console.error('ReactNode > appendChild > node:', this.toString(), 'projectedContent:', projectedContent.toString().trim()); }
230+
if (DEBUG) {
231+
console.error(
232+
'ReactNode > appendChild > node:',
233+
this.toString(),
234+
'projectedContent:',
235+
projectedContent.toString().trim()
236+
);
237+
}
224238
this.childrenToAppend.push(projectedContent);
225239
}
226240

@@ -239,5 +253,4 @@ export class ReactNode {
239253

240254
return '[undefined ReactNode]';
241255
}
242-
243256
}

‎libs/core/src/renderer/renderer.ts‎

Lines changed: 109 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { Injectable, Renderer2, RendererStyleFlags2, RendererType2 } from '@angular/core';
44
import { EventManager, ɵDomRendererFactory2, ɵDomSharedStylesHost } from '@angular/platform-browser';
5+
import * as ReactDOM from 'react-dom';
56

67
import { isReactNode, ReactNode } from './react-node';
78

@@ -25,7 +26,7 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
2526
// renders are flushed.
2627
public setRenderPendingCallback = () => {
2728
this.isRenderPending = true;
28-
}
29+
};
2930

3031
constructor(eventManager: EventManager, sharedStylesHost: ɵDomSharedStylesHost) {
3132
super(eventManager, sharedStylesHost);
@@ -41,13 +42,33 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
4142
return super.createRenderer(element, type);
4243
}
4344

44-
begin() { }
45+
begin() {}
4546

4647
end() {
47-
if (DEBUG) { console.log('RootRenderer > end > isRenderPending:', this.isRenderPending, 'reactRootNodes:', this.reactRootNodes); }
48+
if (DEBUG) {
49+
console.log(
50+
'RootRenderer > end > isRenderPending:',
51+
this.isRenderPending,
52+
'reactRootNodes:',
53+
this.reactRootNodes
54+
);
55+
}
4856
// Flush any pending React element render updates. This cannot be done
4957
// earlier (as is done for DOM elements) because React element props
5058
// are ReadOnly.
59+
60+
// Workaround for ReactNodes inside ReactContent being added to the root of the VDOM and not removed from the VDOM when unmounted from the DOM.
61+
for (let i = 0; i < this.reactRootNodes.length; i++) {
62+
const node = this.reactRootNodes[i];
63+
if (
64+
!isReactNode(node.parent) &&
65+
!document.contains(node.parent) &&
66+
ReactDOM.unmountComponentAtNode(node.parent)
67+
) {
68+
this.reactRootNodes.splice(i--, 1);
69+
}
70+
}
71+
5172
if (this.isRenderPending) {
5273
// Remove root nodes that are pending destroy after render.
5374
this.reactRootNodes = this.reactRootNodes.filter(node => !node.render().destroyPending);
@@ -59,27 +80,35 @@ export class AngularReactRendererFactory extends ɵDomRendererFactory2 {
5980
class ReactRenderer implements Renderer2 {
6081
readonly data: { [key: string]: any } = Object.create(null);
6182

62-
constructor(private rootRenderer: AngularReactRendererFactory) { }
83+
constructor(private rootRenderer: AngularReactRendererFactory) {}
6384

64-
destroy(): void { }
85+
destroy(): void {}
6586

6687
destroyNode(node: ReactNode): void {
67-
if (DEBUG) { console.error('Renderer > destroyNode > node:', node.toString()); }
88+
if (DEBUG) {
89+
console.error('Renderer > destroyNode > node:', node.toString());
90+
}
6891
node.destroyNode();
6992
}
7093

7194
createElement(name: string, namespace?: string): ReactNode {
72-
if (DEBUG) { console.error('Renderer > createElement > name:', name, namespace ? 'namespace:' : '', namespace); }
95+
if (DEBUG) {
96+
console.error('Renderer > createElement > name:', name, namespace ? 'namespace:' : '', namespace);
97+
}
7398
return new ReactNode(name);
7499
}
75100

76101
createComment(value: string): ReactNode {
77-
if (DEBUG) { console.error('Renderer > createComment > value:', value.trim()); }
102+
if (DEBUG) {
103+
console.error('Renderer > createComment > value:', value.trim());
104+
}
78105
return new ReactNode().asComment(value);
79106
}
80107

81108
createText(value: string): ReactNode {
82-
if (DEBUG) { console.error('Renderer > createText > value:', value.trim()); }
109+
if (DEBUG) {
110+
console.error('Renderer > createText > value:', value.trim());
111+
}
83112
return new ReactNode().asText(value);
84113
}
85114

@@ -97,14 +126,18 @@ class ReactRenderer implements Renderer2 {
97126
// Provide a parent element reference to the ReactNode. This will be used later
98127
// once the ReactNode is fully defined and it is subsequently rendered.
99128
if (!isReactNode(parent)) {
100-
if (DEBUG) { console.warn('Renderer > appendChild > asDOM > parent:', parent.toString(), 'node:', node.toString()); }
129+
if (DEBUG) {
130+
console.warn('Renderer > appendChild > asDOM > parent:', parent.toString(), 'node:', node.toString());
131+
}
101132
node.setRenderPendingCallback = this.rootRenderer.setRenderPendingCallback;
102133
this.rootRenderer.reactRootNodes.push(node);
103134
node.parent = parent;
104135
return;
105136
}
106137

107-
if (DEBUG) { console.warn('Renderer > appendChild > asReact > parent:', parent.toString(), 'node:', node.toString()); }
138+
if (DEBUG) {
139+
console.warn('Renderer > appendChild > asReact > parent:', parent.toString(), 'node:', node.toString());
140+
}
108141
node.setRenderPendingCallback = () => parent.setRenderPending();
109142
parent.addChild(node);
110143
node.parent = parent;
@@ -120,7 +153,16 @@ class ReactRenderer implements Renderer2 {
120153
// once the ReactNode is fully defined and it is subsequently rendered. In this
121154
// case, React cannot "insertBefore". Instead, we have to create a target element
122155
// where the ReactNode can be rendered later.
123-
if (DEBUG) { console.warn('Renderer > insertBefore > asDOM > parent:', parent.toString(), 'node:', node.toString(), 'refChild:', refChild.toString()); }
156+
if (DEBUG) {
157+
console.warn(
158+
'Renderer > insertBefore > asDOM > parent:',
159+
parent.toString(),
160+
'node:',
161+
node.toString(),
162+
'refChild:',
163+
refChild.toString()
164+
);
165+
}
124166
const target = document.createElement('div');
125167
parent.insertBefore(target, refChild);
126168
node.parent = target;
@@ -136,39 +178,71 @@ class ReactRenderer implements Renderer2 {
136178
// Remove a parent element reference from the ReactNode. This will be later
137179
// result in the ReactNode unloading itself.
138180
if (!isReactNode(parent)) {
139-
if (DEBUG) { console.warn('Renderer > removeChild > asDOM > parent:', parent.toString(), 'node:', node.toString()); }
181+
if (DEBUG) {
182+
console.warn('Renderer > removeChild > asDOM > parent:', parent.toString(), 'node:', node.toString());
183+
}
140184
node.parent = null;
141185
return;
142186
}
143187

144-
if (DEBUG) { console.warn('Renderer > removeChild > asReact > parent:', parent.toString(), 'node:', node.toString()); }
188+
if (DEBUG) {
189+
console.warn('Renderer > removeChild > asReact > parent:', parent.toString(), 'node:', node.toString());
190+
}
145191
parent.removeChild(node);
146192
}
147193

148194
selectRootElement(selectorOrNode: string | any): any {
149-
if (DEBUG) { console.log('NOT IMPLEMENTED - Renderer > selectRootElement > selectorOrNode:', selectorOrNode); }
195+
if (DEBUG) {
196+
console.log('NOT IMPLEMENTED - Renderer > selectRootElement > selectorOrNode:', selectorOrNode);
197+
}
150198
}
151199

152200
parentNode(node: ReactNode): any {
153-
if (DEBUG) { console.log('NOT IMPLEMENTED - Renderer > parentNode > node:', node.toString()); }
201+
if (DEBUG) {
202+
console.log('NOT IMPLEMENTED - Renderer > parentNode > node:', node.toString());
203+
}
154204
}
155205

156206
nextSibling(node: any): any {
157-
if (DEBUG) { console.log('NOT IMPLEMENTED - Renderer > nextSibling > node:', node.toString()); }
207+
if (DEBUG) {
208+
console.log('NOT IMPLEMENTED - Renderer > nextSibling > node:', node.toString());
209+
}
158210
}
159211

160212
setAttribute(node: ReactNode, name: string, value: string, namespace?: string): void {
161-
if (DEBUG) { console.log('Renderer > setAttribute > node:', node.toString(), 'name:', name, 'value:', value, namespace ? 'namespace:' : '', namespace); }
213+
if (DEBUG) {
214+
console.log(
215+
'Renderer > setAttribute > node:',
216+
node.toString(),
217+
'name:',
218+
name,
219+
'value:',
220+
value,
221+
namespace ? 'namespace:' : '',
222+
namespace
223+
);
224+
}
162225
node.setProperty(name, value);
163226
}
164227

165228
removeAttribute(node: ReactNode, name: string, namespace?: string): void {
166-
if (DEBUG) { console.log('Renderer > removeAttribute > node:', node.toString(), 'name:', name, namespace ? 'namespace:' : '', namespace); }
229+
if (DEBUG) {
230+
console.log(
231+
'Renderer > removeAttribute > node:',
232+
node.toString(),
233+
'name:',
234+
name,
235+
namespace ? 'namespace:' : '',
236+
namespace
237+
);
238+
}
167239
node.removeProperty(name);
168240
}
169241

170242
addClass(node: ReactNode, name: string): void {
171-
if (DEBUG) { console.log('Renderer > addClass > node:', node.toString(), 'name:', name); }
243+
if (DEBUG) {
244+
console.log('Renderer > addClass > node:', node.toString(), 'name:', name);
245+
}
172246

173247
// Update the virtual node.
174248
// TODO: This may only support a single class name, but might work if property name is a single
@@ -177,7 +251,9 @@ class ReactRenderer implements Renderer2 {
177251
}
178252

179253
removeClass(node: ReactNode, name: string): void {
180-
if (DEBUG) { console.log('Renderer > removeClass > node:', node.toString(), 'name:', name); }
254+
if (DEBUG) {
255+
console.log('Renderer > removeClass > node:', node.toString(), 'name:', name);
256+
}
181257

182258
// Update the virtual node.
183259
// TODO: This may not work correctly to remove a single name from a comma-delimited list.
@@ -194,31 +270,37 @@ class ReactRenderer implements Renderer2 {
194270
}
195271

196272
removeStyle(node: ReactNode, style: string, flags: RendererStyleFlags2): void {
197-
if (DEBUG) { console.log('Renderer > removeStyle > node:', node.toString(), 'style:', style, 'flags:', flags); }
273+
if (DEBUG) {
274+
console.log('Renderer > removeStyle > node:', node.toString(), 'style:', style, 'flags:', flags);
275+
}
198276
node.removeProperty('style', style);
199277
}
200278

201279
setProperty(node: ReactNode, name: string, value: any): void {
202-
if (DEBUG) { console.log('Renderer > setProperty > node:', node.toString(), 'name:', name, 'value:', value); }
280+
if (DEBUG) {
281+
console.log('Renderer > setProperty > node:', node.toString(), 'name:', name, 'value:', value);
282+
}
203283
node.setProperty(name, value);
204284
}
205285

206286
setValue(node: ReactNode, value: string): void {
207-
if (DEBUG) { console.log('Renderer > setValue > node:', node.toString(), 'value:', value); }
287+
if (DEBUG) {
288+
console.log('Renderer > setValue > node:', node.toString(), 'value:', value);
289+
}
208290
node.setProperty('value', value);
209291
}
210292

211293
listen(target: ReactNode, event: string, callback: (event: any) => boolean): () => void {
212-
if (DEBUG) { console.log('Renderer > listen > target:', target, 'event:', event); }
294+
if (DEBUG) {
295+
console.log('Renderer > listen > target:', target, 'event:', event);
296+
}
213297
target.setProperty(event, callback);
214298

215299
// NEEDS WORK: Implement prevent default callback behavior.
216300
// return <() => void>this.eventManager.addEventListener(
217301
// target, event, decoratePreventDefault(callback)) as() => void;
218302

219-
220303
// tslint:disable-next-line:no-unused-expression
221304
return () => null;
222305
}
223-
224306
}

0 commit comments

Comments
 (0)