Skip to content

Commit

Permalink
fix(core): mem leaks w/orphan DOM elements when disposing (#153)
Browse files Browse the repository at this point in the history 
* fix(core): mem leaks w/orphan DOM elements when disposing
  • Loading branch information
@ghiscoding
ghiscoding authored Nov 6, 2020
1 parent 6095c7c commit faba5a6
Show file tree
Hide file tree
Showing 36 changed files with 207 additions and 94 deletions.
5 changes: 5 additions & 0 deletions examples/webpack-demo-vanilla-bundle/src/examples/example08.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ export class Example08 {
this.sgb2 = new Slicker.GridBundle(gridContainerElm2, this.columnDefinitions2, { ...ExampleGridOptions, ...this.gridOptions2 }, this.dataset2);
}

dispose() {
this.sgb1?.dispose();
this.sgb2?.dispose();
}

definedGrid1() {
this.columnDefinitions1 = [
{ id: 'title', name: 'Title', field: 'title', sortable: true, columnGroup: 'Common Factor' },
Expand Down
12 changes: 8 additions & 4 deletions packages/common/src/editors/autoCompleteEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class AutoCompleteEditor implements Editor {
private _currentValue: any;
private _defaultTextValue: string;
private _elementCollection: any[];
private _lastInputKeyEvent: JQueryEventObject;
private _lastInputKeyEvent: JQuery.Event;

/** The JQuery DOM element */
private _$editorElm: any;
Expand Down Expand Up @@ -154,7 +154,11 @@ export class AutoCompleteEditor implements Editor {
}

destroy() {
this._$editorElm.off('keydown.nav').remove();
if (this._$editorElm) {
this._$editorElm.autocomplete('destroy');
this._$editorElm.off('keydown.nav').remove();
this._$editorElm = null;
}
}

/**
Expand Down Expand Up @@ -193,7 +197,7 @@ export class AutoCompleteEditor implements Editor {
}

focus() {
this._$editorElm.focus().select();
this._$editorElm?.focus().select();
}

show() {
Expand Down Expand Up @@ -443,7 +447,7 @@ export class AutoCompleteEditor implements Editor {

this._$editorElm = $(`<input type="text" role="presentation" autocomplete="off" class="autocomplete editor-text editor-${columnId}" placeholder="${placeholder}" title="${title}" />`)
.appendTo(this.args.container)
.on('keydown.nav', (event: JQueryEventObject) => {
.on('keydown.nav', (event: JQuery.Event) => {
this._lastInputKeyEvent = event;
if (event.keyCode === KeyCode.LEFT || event.keyCode === KeyCode.RIGHT) {
event.stopImmediatePropagation();
Expand Down
19 changes: 12 additions & 7 deletions packages/common/src/editors/checkboxEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ declare const Slick: SlickNamespace;
* KeyDown events are also handled to provide handling for Tab, Shift-Tab, Esc and Ctrl-Enter.
*/
export class CheckboxEditor implements Editor {
private _input: HTMLInputElement;
private _input: HTMLInputElement | null;
private _checkboxContainerElm: HTMLDivElement;
private _originalValue?: boolean | string;

Expand Down Expand Up @@ -97,6 +97,7 @@ export class CheckboxEditor implements Editor {
if (this._input?.remove) {
this._input.remove();
}
this._input = null;
}

disable(isDisabled = true) {
Expand All @@ -122,7 +123,9 @@ export class CheckboxEditor implements Editor {
}

focus(): void {
this._input.focus();
if (this._input) {
this._input.focus();
}
}

show() {
Expand All @@ -134,12 +137,14 @@ export class CheckboxEditor implements Editor {
}

getValue() {
return this._input.checked;
return this._input?.checked ?? false;
}

setValue(val: boolean | string, isApplyingValue = false) {
const isChecked = val ? true : false;
this._input.checked = isChecked;
if (this._input) {
this._input.checked = isChecked;
}

if (isApplyingValue) {
this.applyValue(this.args.item, this.serializeValue());
Expand Down Expand Up @@ -177,7 +182,7 @@ export class CheckboxEditor implements Editor {
loadValue(item: any) {
const fieldName = this.columnDef && this.columnDef.field;

if (item && fieldName !== undefined) {
if (item && fieldName !== undefined && this._input) {
// is the field a complex object, "address.streetNumber"
const isComplexObject = fieldName?.indexOf('.') > 0;
const value = (isComplexObject) ? getDescendantProperty(item, fieldName) : item[fieldName];
Expand All @@ -201,12 +206,12 @@ export class CheckboxEditor implements Editor {
}

serializeValue(): boolean {
return this._input.checked;
return this._input?.checked ?? false;
}

validate(_targetElm?: null, inputValue?: any): EditorValidationResult {
const isRequired = this.args?.compositeEditorOptions ? false : this.columnEditor.required;
const isChecked = (inputValue !== undefined) ? inputValue : this._input.checked;
const isChecked = (inputValue !== undefined) ? inputValue : this._input?.checked;
const errorMsg = this.columnEditor.errorMessage;

// when using Composite Editor, we also want to recheck if the field if disabled/enabled since it might change depending on other inputs on the composite form
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/editors/dateEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,15 @@ export class DateEditor implements Editor {
this._$input.remove();
if (this._$editorInputElm?.remove) {
this._$editorInputElm.remove();
this._$editorInputElm = null;
}
if (this._$inputWithData && typeof this._$inputWithData.remove === 'function') {
this._$inputWithData.remove();
this._$inputWithData = null;
}
if (this.flatInstance && typeof this.flatInstance.destroy === 'function') {
this.flatInstance.destroy();
this.flatInstance = null;
}
}

Expand Down
43 changes: 26 additions & 17 deletions packages/common/src/editors/floatEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ declare const Slick: SlickNamespace;
* KeyDown events are also handled to provide handling for Tab, Shift-Tab, Esc and Ctrl-Enter.
*/
export class FloatEditor implements Editor {
private _input: HTMLInputElement;
private _input: HTMLInputElement | null;
private _lastInputKeyEvent: KeyboardEvent;
private _originalValue: number | string;

Expand Down Expand Up @@ -80,7 +80,7 @@ export class FloatEditor implements Editor {
cellContainer.appendChild(this._input);
}

this._input.onfocus = () => this._input.select();
this._input.onfocus = () => this._input?.select();
this._input.onkeydown = ((event: KeyboardEvent) => {
this._lastInputKeyEvent = event;
if (event.keyCode === KeyCode.LEFT || event.keyCode === KeyCode.RIGHT) {
Expand Down Expand Up @@ -108,7 +108,12 @@ export class FloatEditor implements Editor {
this._input.removeEventListener('keyup', this.handleOnKeyUp);
this._input.removeEventListener('change', this.handleOnChange);
this._input.removeEventListener('wheel', this.handleOnChange.bind(this));
setTimeout(() => this._input.remove());
setTimeout(() => {
if (this._input) {
this._input.remove();
this._input = null;
}
});
}
}

Expand All @@ -133,7 +138,9 @@ export class FloatEditor implements Editor {
}

focus(): void {
this._input.focus();
if (this._input) {
this._input.focus();
}
}

show() {
Expand Down Expand Up @@ -168,19 +175,21 @@ export class FloatEditor implements Editor {
}

getValue(): string {
return this._input.value || '';
return this._input?.value || '';
}

setValue(value: number | string, isApplyingValue = false) {
this._input.value = `${value}`;
if (this._input) {
this._input.value = `${value}`;

if (isApplyingValue) {
this.applyValue(this.args.item, this.serializeValue());
if (isApplyingValue) {
this.applyValue(this.args.item, this.serializeValue());

// if it's set by a Composite Editor, then also trigger a change for it
const compositeEditorOptions = this.args.compositeEditorOptions;
if (compositeEditorOptions) {
this.handleChangeOnCompositeEditor(null, compositeEditorOptions);
// if it's set by a Composite Editor, then also trigger a change for it
const compositeEditorOptions = this.args.compositeEditorOptions;
if (compositeEditorOptions) {
this.handleChangeOnCompositeEditor(null, compositeEditorOptions);
}
}
}
}
Expand All @@ -203,7 +212,7 @@ export class FloatEditor implements Editor {
}

isValueChanged(): boolean {
const elmValue = this._input.value;
const elmValue = this._input?.value;
const lastKeyEvent = this._lastInputKeyEvent && this._lastInputKeyEvent.keyCode;
if (this.columnEditor && this.columnEditor.alwaysSaveOnEnterKey && lastKeyEvent === KeyCode.ENTER) {
return true;
Expand All @@ -216,7 +225,7 @@ export class FloatEditor implements Editor {

if (fieldName !== undefined) {

if (item && fieldName !== undefined) {
if (item && fieldName !== undefined && this._input) {
// is the field a complex object, "address.streetNumber"
const isComplexObject = fieldName?.indexOf('.') > 0;
const value = (isComplexObject) ? getDescendantProperty(item, fieldName) : item[fieldName];
Expand Down Expand Up @@ -246,8 +255,8 @@ export class FloatEditor implements Editor {
}

serializeValue() {
const elmValue = this._input.value;
if (elmValue === '' || isNaN(+elmValue)) {
const elmValue = this._input?.value;
if (elmValue === undefined || elmValue === '' || isNaN(+elmValue)) {
return elmValue;
}

Expand All @@ -271,7 +280,7 @@ export class FloatEditor implements Editor {
return { valid: true, msg: '' };
}

const elmValue = (inputValue !== undefined) ? inputValue : this._input && this._input.value;
const elmValue = (inputValue !== undefined) ? inputValue : this._input?.value;
return floatValidator(elmValue, {
editorArgs: this.args,
errorMessage: this.columnEditor.errorMessage,
Expand Down
41 changes: 25 additions & 16 deletions packages/common/src/editors/integerEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ declare const Slick: SlickNamespace;
*/
export class IntegerEditor implements Editor {
private _lastInputKeyEvent: KeyboardEvent;
private _input: HTMLInputElement;
private _input: HTMLInputElement | null;
private _originalValue: number | string;

/** is the Editor disabled? */
Expand Down Expand Up @@ -78,7 +78,7 @@ export class IntegerEditor implements Editor {
cellContainer.appendChild(this._input);
}

this._input.onfocus = () => this._input.select();
this._input.onfocus = () => this._input?.select();
this._input.onkeydown = ((event: KeyboardEvent) => {
this._lastInputKeyEvent = event;
if (event.keyCode === KeyCode.LEFT || event.keyCode === KeyCode.RIGHT) {
Expand Down Expand Up @@ -106,7 +106,12 @@ export class IntegerEditor implements Editor {
this._input.removeEventListener('keyup', this.handleOnKeyUp);
this._input.removeEventListener('change', this.handleOnChange);
this._input.removeEventListener('wheel', this.handleOnChange);
setTimeout(() => this._input.remove());
setTimeout(() => {
if (this._input) {
this._input.remove();
this._input = null;
}
});
}
}

Expand All @@ -131,7 +136,9 @@ export class IntegerEditor implements Editor {
}

focus(): void {
this._input.focus();
if (this._input) {
this._input.focus();
}
}

show() {
Expand All @@ -143,19 +150,21 @@ export class IntegerEditor implements Editor {
}

getValue(): string {
return this._input.value || '';
return this._input?.value || '';
}

setValue(value: number | string, isApplyingValue = false) {
this._input.value = `${value}`;
if (this._input) {
this._input.value = `${value}`;

if (isApplyingValue) {
this.applyValue(this.args.item, this.serializeValue());
if (isApplyingValue) {
this.applyValue(this.args.item, this.serializeValue());

// if it's set by a Composite Editor, then also trigger a change for it
const compositeEditorOptions = this.args.compositeEditorOptions;
if (compositeEditorOptions) {
this.handleChangeOnCompositeEditor(null, compositeEditorOptions);
// if it's set by a Composite Editor, then also trigger a change for it
const compositeEditorOptions = this.args.compositeEditorOptions;
if (compositeEditorOptions) {
this.handleChangeOnCompositeEditor(null, compositeEditorOptions);
}
}
}
}
Expand All @@ -179,7 +188,7 @@ export class IntegerEditor implements Editor {
}

isValueChanged(): boolean {
const elmValue = this._input.value;
const elmValue = this._input?.value;
const lastKeyEvent = this._lastInputKeyEvent && this._lastInputKeyEvent.keyCode;
if (this.columnEditor && this.columnEditor.alwaysSaveOnEnterKey && lastKeyEvent === KeyCode.ENTER) {
return true;
Expand All @@ -190,7 +199,7 @@ export class IntegerEditor implements Editor {
loadValue(item: any) {
const fieldName = this.columnDef && this.columnDef.field;

if (item && fieldName !== undefined) {
if (item && fieldName !== undefined && this._input) {
// is the field a complex object, "address.streetNumber"
const isComplexObject = fieldName?.indexOf('.') > 0;

Expand All @@ -215,8 +224,8 @@ export class IntegerEditor implements Editor {
}

serializeValue() {
const elmValue = this._input.value;
if (elmValue === '' || isNaN(+elmValue)) {
const elmValue = this._input?.value;
if (elmValue === undefined || elmValue === '' || isNaN(+elmValue)) {
return elmValue;
}
const output = isNaN(+elmValue) ? elmValue : parseInt(elmValue, 10);
Expand Down
13 changes: 11 additions & 2 deletions packages/common/src/editors/longTextEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,23 @@ export class LongTextEditor implements Editor {
}

destroy() {
this._$wrapper.remove();
if (this._$textarea) {
this._$textarea.off('keydown');
this._$textarea.off('keyup');
}
if (this._$wrapper) {
this._$wrapper.find('.btn-save').off('click');
this._$wrapper.find('.btn-cancel').off('click');
this._$wrapper.remove();
}
this._$wrapper = null;
}

disable(isDisabled = true) {
const prevIsDisabled = this.disabled;
this.disabled = isDisabled;

if (this._$textarea) {
if (this._$textarea && this._$wrapper) {
if (isDisabled) {
this._$textarea.attr('disabled', 'disabled');
this._$wrapper.addClass('disabled');
Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/editors/selectEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,11 @@ export class SelectEditor implements Editor {
this._destroying = true;
if (this.$editorElm && typeof this.$editorElm.multipleSelect === 'function') {
this.$editorElm.multipleSelect('destroy');
this.$editorElm.remove();
const elementClassName = this.elementName.toString().replace('.', '\\.'); // make sure to escape any dot "." from CSS class to avoid console error
$(`[name=${elementClassName}].ms-drop`).remove();
}
if (this.$editorElm && typeof this.$editorElm.remove === 'function') {
this.$editorElm.remove();
this.$editorElm = null;
}
}

Expand Down
Loading

0 comments on commit faba5a6

Please sign in to comment.