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

Resolved https://github.com/surveyjs/survey-creator/issues/4484 - memory leaks in survey creator #4508

Merged
merged 14 commits into from
Sep 5, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export class ItemValueDesignerComponent extends CreatorModelComponent<ItemValueW
protected getModel(): ItemValueWrapperViewModel {
return this.adorner;
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.adorner.dispose();
}
}

AngularComponentFactory.Instance.registerComponent("svc-item-value", ItemValueDesignerComponent);
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ export class MatrixCellComponent extends CreatorModelComponent<MatrixCellWrapper
protected getModel(): MatrixCellWrapperViewModel {
return this.adorner;
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.adorner.dispose();
}
}

AngularComponentFactory.Instance.registerComponent("svc-matrix-cell", MatrixCellComponent);
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export class QuestionRatingAdornerDesignerComponent extends CreatorModelComponen
protected getModel(): QuestionRatingAdornerViewModel {
return this.adorner;
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.adorner.dispose();
}
}

AngularComponentFactory.Instance.registerComponent("svc-rating-question-content", QuestionRatingAdornerDesignerComponent);
Expand Down
1 change: 0 additions & 1 deletion packages/survey-creator-angular/src/page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export class PageDesignerComponent extends CreatorModelComponent<PageAdorner> {
public adorner!: PageAdorner;
protected createModel(): void {
if (this.model) {
this.previousModel?.dispose();
this.adorner = new PageAdorner(this.creator, this.model);
}
}
Expand Down
4 changes: 4 additions & 0 deletions packages/survey-creator-angular/src/question.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ export class QuestionDesignerComponent extends CreatorModelComponent<QuestionAdo
event.stopPropagation();
this.adorner.addNewQuestion();
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.adorner.dispose();
}
adornerComponent = "";
}

Expand Down
5 changes: 5 additions & 0 deletions packages/survey-creator-angular/src/row.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export class CreatorRowComponent extends CreatorModelComponent<RowViewModel> {
getPropertiesToTrack(): string[] {
return ["creator", "row"];
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.model.dispose();
}

}

AngularComponentFactory.Instance.registerComponent("svc-row", CreatorRowComponent);
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ export class StringEditorComponent extends CreatorModelComponent<StringEditorVie
}
}
override ngOnDestroy(): void {
this.baseModel.blurEditor = undefined as any;
this.baseModel.getEditorElement = undefined as any;
this.baseModel.dispose();
this.locString?.onStringChanged.remove(this.onChangeHandler);
super.ngOnDestroy();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,8 @@ export class ToolboxToolComponent extends CreatorModelComponent<Action> {
protected override getPropertiesToUpdateSync(): string[] {
return ["mode"];
}
override ngOnDestroy(): void {
super.ngOnDestroy();
this.model.dispose();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export class SurveyElementAdornerBase<T extends SurveyElement = SurveyElement> e
public dispose(): void {
super.dispose();
this.detachElement(this.surveyElement);
this.actionContainer.dispose();
this.creator.sidebar.onPropertyChanged.remove(this.sidebarFlyoutModeChangedFunc);
this.selectedPropPageFunc = undefined;
this.sidebarFlyoutModeChangedFunc = undefined;
}
protected onElementSelectedChanged(isSelected: boolean): void {
if (!isSelected) return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ export class PageNavigatorViewModel extends Base {
};
private _resizeObserver: ResizeObserver;

private pcPropertyChangedHandler = (sender, options) => {
if (options.name === "toolboxLocation") {
if (this.pagesController.creator["toolboxLocation"] == "sidebar") {
this.popupModel.horizontalPosition = "right";
} else {
this.popupModel.horizontalPosition = this.pagesController.creator["toolboxLocation"];
}
}
}

constructor(private pagesController: PagesController, private pageEditMode: string) {
super();
this.icon = "icon-select-page";
Expand All @@ -38,15 +48,7 @@ export class PageNavigatorViewModel extends Base {
};
this.popupModel.onHide = () => { this.isPopupOpened = false; };
if (!!this.pagesController.creator["onPropertyChanged"]) {
this.pagesController.creator["onPropertyChanged"].add((sender, options) => {
if (options.name === "toolboxLocation") {
if (this.pagesController.creator["toolboxLocation"] == "sidebar") {
this.popupModel.horizontalPosition = "right";
} else {
this.popupModel.horizontalPosition = this.pagesController.creator["toolboxLocation"];
}
}
});
this.pagesController.creator["onPropertyChanged"].add(this.pcPropertyChangedHandler);
}
this.buildItems();
}
Expand All @@ -55,6 +57,18 @@ export class PageNavigatorViewModel extends Base {
this.stopItemsContainerHeightObserver();
this.pagesController.onPagesChanged.remove(this.pagesChangedFunc);
this.pagesController.onCurrentPageChanged.remove(this.currentPagesChangedFunc);
if (!!this.pagesController.creator["onPropertyChanged"]) {
this.pagesController.creator["onPropertyChanged"].remove(this.pcPropertyChangedHandler);
this.pcPropertyChangedHandler = undefined;
}
if(this.pageListModel) {
this.pageListModel.dispose();
}
if(this.popupModel) {
this.popupModel.dispose();
}
this._scrollableContainer = undefined;
this._itemsContainer = undefined;
}

@propertyArray() items: Array<IAction>;
Expand Down
4 changes: 2 additions & 2 deletions packages/survey-creator-core/src/components/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class PageAdorner extends SurveyElementAdornerBase<PageModel> {
@property({ defaultValue: "" }) currentAddQuestionType: string;
@property({ defaultValue: null }) dragTypeOverMe: DragTypeOverMeEnum;
private updateDragTypeOverMe() {
this.dragTypeOverMe = this.page.dragTypeOverMe;
this.dragTypeOverMe = this.page?.dragTypeOverMe;
}
constructor(creator: CreatorBase, page: PageModel) {
super(creator, page);
Expand Down Expand Up @@ -93,8 +93,8 @@ export class PageAdorner extends SurveyElementAdornerBase<PageModel> {
};
}
public dispose(): void {
super.dispose();
this.detachElement(this.page);
super.dispose();
this.onPropertyValueChangedCallback = undefined;
}
public get isGhost(): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class QuestionImageAdornerViewModel extends QuestionAdornerViewModel {
this.filePresentationModel.storeDataAsText = false;
surveyModel.onUploadFiles.add((s, o) => {
const fileToUpload = o.files[0];
if(!!fileToUpload) {
if (!!fileToUpload) {
this.creator.uploadFiles(o.files, this.question, (status, link) => {
this.question.imageLink = link;
});
Expand Down Expand Up @@ -78,4 +78,9 @@ export class QuestionImageAdornerViewModel extends QuestionAdornerViewModel {
public get chooseImageText(): string {
return getLocString("ed.imageChooseImage");
}

public dispose() {
super.dispose();
this.questionRoot = undefined;
}
}
1 change: 1 addition & 0 deletions packages/survey-creator-core/src/components/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ export class QuestionAdornerViewModel extends SurveyElementAdornerBase {
if (!!this.surveyElement["setCanShowOptionItemCallback"]) {
(<any>this.surveyElement).setCanShowOptionItemCallback(undefined);
}
super.dispose();
}
get isDraggable() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ export class SidebarModel extends Base {
private getCurrentHandles(): string {
return this.creator.sidebarLocation == "right" ? "w" : "e";
}
private sidebarLocationChangedHandler = (sender, options) => {
if (options.name === "sidebarLocation" && !!this.resizeManager) {
this.resizeManager.setHandles(this.getCurrentHandles());
}
};

constructor(private creator: CreatorBase) {
super();
Expand All @@ -99,11 +104,7 @@ export class SidebarModel extends Base {
this.visible = options.show;
};
this.creator.onShowSidebarVisibilityChanged.add(this.onSidebarVisibilityChanged);
this.creator.onPropertyChanged.add((sender, options) => {
if (options.name === "sidebarLocation" && !!this.resizeManager) {
this.resizeManager.setHandles(this.getCurrentHandles());
}
});
this.creator.onPropertyChanged.add(this.sidebarLocationChangedHandler);
this.visible = this.creator.showSidebar;
this.createActions();
}
Expand Down Expand Up @@ -131,7 +132,10 @@ export class SidebarModel extends Base {
public dispose() {
if (!!this.creator && !this.isDisposed) {
this.creator.onShowSidebarVisibilityChanged.remove(this.onSidebarVisibilityChanged);
this.creator.onPropertyChanged.remove(this.sidebarLocationChangedHandler);
this.sidebarLocationChangedHandler = undefined;
}
this.resetResizeManager();
super.dispose();
}
public initResizeManager(container: HTMLDivElement): void {
Expand Down
27 changes: 16 additions & 11 deletions packages/survey-creator-core/src/components/string-editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export abstract class StringItemsNavigatorBase {

protected addNewItems(items: any, startIndex: number, itemsToAdd: string[]) {
const createNewItem = (val: any): ItemValue => {
if(this.question.createItemValue) return this.question.createItemValue(val);
if (this.question.createItemValue) return this.question.createItemValue(val);
return new ItemValue(val);
};
let newItems = items.slice(0, startIndex).concat(itemsToAdd.map(text => createNewItem(text))).concat(items.slice(startIndex + 1));
Expand Down Expand Up @@ -117,7 +117,7 @@ class StringItemsNavigatorSelectBase extends StringItemsNavigatorBase {
}
protected addNewItem(creator: CreatorBase, items: any, text: string = null) {
const itemValue = creator.createNewItemValue(this.question);
if(!!text) itemValue.value = text;
if (!!text) itemValue.value = text;
}
protected getItemsPropertyName(items: any) {
return "choices";
Expand Down Expand Up @@ -217,9 +217,7 @@ export class StringEditorViewModelBase extends Base {

constructor(private locString: LocalizableString, private creator: CreatorBase) {
super();
this.connector = StringEditorConnector.get(locString);
this.connector.onDoActivate.add(() => { this.activate(); });
this.checkMarkdownToTextConversion(this.locString.owner, this.locString.name);
this.setLocString(locString);
}

public afterRender() {
Expand All @@ -228,7 +226,12 @@ export class StringEditorViewModelBase extends Base {
}
}

public activate() {
public dispose() {
super.dispose();
this.connector.onDoActivate.remove(this.activate);
}

public activate = () => {
const element = this.getEditorElement();
if (element && element.offsetParent != null) {
element.focus();
Expand All @@ -239,9 +242,11 @@ export class StringEditorViewModelBase extends Base {
}

public setLocString(locString: LocalizableString) {
this.connector?.onDoActivate.clear();
this.locString = locString;
this.connector = StringEditorConnector.get(locString);
this.connector.onDoActivate.add(() => { this.activate(); });
this.connector.onDoActivate.add(this.activate);
this.checkMarkdownToTextConversion(this.locString.owner, this.locString.name);
}
public checkConstraints(event: any) {
if (this.maxLength > 0 && event.keyCode >= 32) {
Expand Down Expand Up @@ -373,12 +378,12 @@ export class StringEditorViewModelBase extends Base {
this.creator.inplaceEditForValues &&
["noneText", "otherText", "selectAllText"].indexOf(this.locString.name) == -1) {
const itemValue = <ItemValue>this.locString.owner;
if(itemValue.value !== clearedText) {
if(!!itemValue.locOwner && !!itemValue.ownerPropertyName) {
if (itemValue.value !== clearedText) {
if (!!itemValue.locOwner && !!itemValue.ownerPropertyName) {
const choices = itemValue.locOwner[itemValue.ownerPropertyName];
if(Array.isArray(choices) && !!ItemValue.getItemByValue(choices, clearedText)) {
if (Array.isArray(choices) && !!ItemValue.getItemByValue(choices, clearedText)) {
clearedText = getNextItemValue(clearedText, choices);
if(!!event && !!event.target) {
if (!!event && !!event.target) {
event.target.innerText = clearedText;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,8 @@ export class TabDesignerPlugin implements ICreatorPlugin {
action: () => {
this.selectSurvey();
},
active: <any>new ComputedUpdater<boolean>(() => {
return notShortCircuitAnd(this.creator.sidebar.activeTab === this.propertyGridTab.id, this.isSurveySelected);
}),
pressed: <any>new ComputedUpdater<boolean>(() => {
return notShortCircuitAnd(this.creator.sidebar.activeTab === this.propertyGridTab.id, this.isSurveySelected);
}),
active: this.isSettingsActive,
pressed: this.isSettingsActive,
visible: this.createVisibleUpdater(),
locTitleName: "ed.surveySettings",
locTooltipName: "ed.surveySettingsTooltip",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export class TabTestPlugin implements ICreatorPlugin {
if (this.model) {
this.simulatorTheme = this.model.simulator.survey.css;
this.model.onSurveyCreatedCallback = undefined;
this.model.dispose();
this.model = undefined;
}
this.languageSelectorAction.visible = false;
Expand Down
6 changes: 6 additions & 0 deletions packages/survey-creator-core/src/components/tabs/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,4 +307,10 @@ export class TestSurveyTabViewModel extends Base {
this.survey.onScroll();
return true;
}
public dispose(): void {
if (this.selectPageAction) {
this.selectPageAction.dispose();
}
this.simulator.dispose();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export class TabThemePlugin implements ICreatorPlugin {
if (this.model) {
this.simulatorTheme = this.model.simulator.survey.css;
this.model.onSurveyCreatedCallback = undefined;
this.model.dispose();
this.model = undefined;
}
this.sidebarTab.visible = false;
Expand Down
7 changes: 7 additions & 0 deletions packages/survey-creator-core/src/components/tabs/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1077,4 +1077,11 @@ export class ThemeSurveyTabViewModel extends Base {

return themeEditorSurveyJSON;
}
public dispose(): void {
this.themeEditorSurveyValue?.dispose();
if (this.selectPageAction) {
this.selectPageAction.dispose();
}
this.simulator.dispose();
}
}
4 changes: 2 additions & 2 deletions packages/survey-creator-knockout/src/adorners/question.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ export function createQuestionViewModel(
return model.isEmptyElement;
});
model["adornerComponent"] = undefined;
new ImplementorBase(model);
const implementor = new ImplementorBase(model);
ko.utils.domNodeDisposal.addDisposeCallback(componentInfo.element, () => {
model.dispose();
implementor.dispose();
});
return model;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ ko.components.register("svc-page-navigator", {
createViewModel: (params: any, componentInfo: any) => {
const model = new PageNavigatorView(params.controller, params.pageEditMode);
model.setItemsContainer(componentInfo.element.parentElement);
new ImplementorBase(model);
const implementor = new ImplementorBase(model);
const scrollableViewPort = componentInfo.element.parentElement.parentElement.parentElement;
model.setScrollableContainer(scrollableViewPort);
if (params.pageEditMode !== "bypage") {
Expand All @@ -38,6 +38,7 @@ ko.components.register("svc-page-navigator", {
ko.utils.domNodeDisposal.addDisposeCallback(componentInfo.element, () => {
scrollableViewPort.onscroll = undefined;
model.dispose();
implementor.dispose();
});
return model;
},
Expand Down
4 changes: 4 additions & 0 deletions packages/survey-creator-knockout/src/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export class CreatorSurveyPageComponent extends PageAdorner {
fixedDispose(): void {
this.pageUpdater && this.pageUpdater.dispose();
super.dispose();
if(ko.isWritableObservable(this._page)) {
(this._page as ko.Observable<PageModel>)(undefined);
}
this._page = undefined;
}

dispose(): void {
Expand Down
Loading