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

Fix/destroy-contexts #569

Merged
merged 7 commits into from
Dec 4, 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
12 changes: 11 additions & 1 deletion abstract/Block.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-check
import { BaseComponent } from '@symbiotejs/symbiote';
import { BaseComponent, Data } from '@symbiotejs/symbiote';
import { EventEmitter } from '../blocks/UploadCtxProvider/EventEmitter.js';
import { createWindowHeightTracker, getIsWindowHeightTracked } from '../utils/createWindowHeightTracker.js';
import { getPluralForm } from '../utils/getPluralForm.js';
Expand Down Expand Up @@ -188,8 +188,18 @@ export class Block extends BaseComponent {
}

destroyCallback() {
/** @type {Set<Block>} */
let blocksRegistry = this.$['*blocksRegistry'];
blocksRegistry.delete(this);

// Destroy local context
// TODO: this should be done inside symbiote
Data.deleteCtx(this);

if (blocksRegistry.size === 0) {
// Destroy external context if there is no any blocks left inside it
Data.deleteCtx(this.ctxName);
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions abstract/TypedCollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export class TypedCollection {

/** @param {Function} handler */
unobserveCollection(handler) {
this.__collectionObservers.delete(handler);
this.__collectionObservers?.delete(handler);
}

/**
Expand Down Expand Up @@ -201,7 +201,7 @@ export class TypedCollection {

/** @param {Function} handler */
unobserveProperties(handler) {
this.__propertyObservers.delete(handler);
this.__propertyObservers?.delete(handler);
}

/**
Expand All @@ -228,7 +228,7 @@ export class TypedCollection {
}

destroy() {
Data.deleteCtx(this.__data);
Data.deleteCtx(this.__ctxId);
this.__propertyObservers = null;
this.__collectionObservers = null;
for (let id in this.__subsMap) {
Expand Down
4 changes: 3 additions & 1 deletion abstract/UploaderBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { TypedCollection } from './TypedCollection.js';
import { uploadEntrySchema } from './uploadEntrySchema.js';

export class UploaderBlock extends ActivityBlock {
couldBeCtxOwner = false;
isCtxOwner = false;

init$ = uploaderBlockCtx(this);
Expand Down Expand Up @@ -84,7 +85,7 @@ export class UploaderBlock extends ActivityBlock {
this.add('*uploadCollection', uploadCollection);
}

if (!this.hasCtxOwner) {
if (!this.hasCtxOwner && this.couldBeCtxOwner) {
this.initCtxOwner();
}
}
Expand All @@ -94,6 +95,7 @@ export class UploaderBlock extends ActivityBlock {
if (this.isCtxOwner) {
this._unobserveCollectionProperties?.();
this._unobserveCollection?.();
this.uploadCollection.destroy();
}
}

Expand Down
1 change: 1 addition & 0 deletions blocks/CameraSource/CameraSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { debounce } from '../utils/debounce.js';
import { UploadSource } from '../utils/UploadSource.js';

export class CameraSource extends UploaderBlock {
couldBeCtxOwner = true;
activityType = ActivityBlock.activities.CAMERA;

/** @private */
Expand Down
2 changes: 1 addition & 1 deletion blocks/CloudImageEditor/src/CloudImageEditorBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { debounce } from '../../utils/debounce.js';
import { CloudImageEditorBase } from './CloudImageEditorBase.js';
import { classNames } from './lib/classNames.js';
import { parseCropPreset } from './lib/parseCropPreset.js';
import { operationsToTransformations, transformationsToOperations } from './lib/transformationUtils.js';
import { parseTabs } from './lib/parseTabs.js';
import { operationsToTransformations, transformationsToOperations } from './lib/transformationUtils.js';
import { initState } from './state.js';
import { TEMPLATE } from './template.js';
import { TabId } from './toolbar-constants.js';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { UploaderBlock } from '../../abstract/UploaderBlock.js';
import { CloudImageEditorBlock } from '../CloudImageEditor/index.js';

export class CloudImageEditorActivity extends UploaderBlock {
couldBeCtxOwner = true;
activityType = ActivityBlock.activities.CLOUD_IMG_EDIT;

constructor() {
Expand Down
5 changes: 0 additions & 5 deletions blocks/Config/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ const attrStateMapping = /** @type {Record<keyof import('../../types').ConfigAtt
});

class ConfigClass extends Block {
ctxOwner = true;
requireCtxName = true;

constructor() {
Expand All @@ -55,10 +54,6 @@ class ConfigClass extends Block {
])
),
};

Object.assign(this, {
test: 'test',
});
}

initCallback() {
Expand Down
23 changes: 17 additions & 6 deletions blocks/DropArea/DropArea.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
// @ts-check

import { Data } from '@symbiotejs/symbiote';
import { ActivityBlock } from '../../abstract/ActivityBlock.js';
import { UploaderBlock } from '../../abstract/UploaderBlock.js';
import { stringToArray } from '../../utils/stringToArray.js';
import { asBoolean } from '../Config/normalizeConfigValue.js';
import { UploadSource } from '../utils/UploadSource.js';
import { DropzoneState, addDropzone } from './addDropzone.js';

const GLOBAL_CTX_NAME = 'lr-drop-area';
const REGISTRY_KEY = `${GLOBAL_CTX_NAME}/registry`;

export class DropArea extends UploaderBlock {
constructor() {
super();
Expand All @@ -20,7 +24,7 @@ export class DropArea extends UploaderBlock {
isEnabled: true,
isVisible: true,
text: this.l10n('drop-files-here'),
'lr-drop-area/targets': null,
[REGISTRY_KEY]: null,
};
}

Expand All @@ -45,10 +49,10 @@ export class DropArea extends UploaderBlock {
initCallback() {
super.initCallback();

if (!this.$['lr-drop-area/targets']) {
this.$['lr-drop-area/targets'] = new Set();
if (!this.$[REGISTRY_KEY]) {
this.$[REGISTRY_KEY] = new Set();
}
this.$['lr-drop-area/targets'].add(this);
this.$[REGISTRY_KEY].add(this);

this.defineAccessor(
'disabled',
Expand Down Expand Up @@ -182,7 +186,7 @@ export class DropArea extends UploaderBlock {
if (!this.$.isFullscreen) {
return false;
}
const otherTargets = [...this.$['lr-drop-area/targets']].filter((el) => el !== this);
const otherTargets = [...this.$[REGISTRY_KEY]].filter((el) => el !== this);
const activeTargets = otherTargets.filter((/** @type {typeof this} */ el) => {
return el.isActive();
});
Expand All @@ -209,7 +213,14 @@ export class DropArea extends UploaderBlock {
destroyCallback() {
super.destroyCallback();

this.$['lr-drop-area/targets']?.remove?.(this);
/** @type {Set<DropArea>} */
const registry = this.$[REGISTRY_KEY];
if (registry) {
registry.delete(this);
if (registry.size === 0) {
Data.deleteCtx(GLOBAL_CTX_NAME);
}
}

this._destroyDropzone?.();
this._destroyContentWrapperDropzone?.();
Expand Down
1 change: 1 addition & 0 deletions blocks/ExternalSource/ExternalSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { queryString } from './query-string.js';
/** @typedef {SelectedFileMessage | EmbedCssMessage} Message */

export class ExternalSource extends UploaderBlock {
couldBeCtxOwner = true;
activityType = ActivityBlock.activities.EXTERNAL;

constructor() {
Expand Down
1 change: 1 addition & 0 deletions blocks/FileItem/FileItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const FileItemState = Object.freeze({
});

export class FileItem extends UploaderBlock {
couldBeCtxOwner = true;
pauseRender = true;

/** @private */
Expand Down
3 changes: 2 additions & 1 deletion blocks/Img/ImgBase.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BaseComponent } from '@symbiotejs/symbiote';
import { BaseComponent, Data } from '@symbiotejs/symbiote';
import { applyTemplateData } from '../../utils/template-utils.js';
import { createCdnUrl, createCdnUrlModifiers, createOriginalUrl } from '../../utils/cdn-utils.js';
import { PROPS_MAP } from './props-map.js';
Expand Down Expand Up @@ -339,6 +339,7 @@ export class ImgBase extends BaseComponent {
});
this._isnObserver = null;
}
Data.deleteCtx(this);
}

static get observedAttributes() {
Expand Down
1 change: 1 addition & 0 deletions blocks/SimpleBtn/SimpleBtn.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { UploaderBlock } from '../../abstract/UploaderBlock.js';
import { asBoolean } from '../Config/normalizeConfigValue.js';

export class SimpleBtn extends UploaderBlock {
couldBeCtxOwner = true;
constructor() {
super();

Expand Down
1 change: 1 addition & 0 deletions blocks/SourceBtn/SourceBtn.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ActivityBlock } from '../../abstract/ActivityBlock.js';
const L10N_PREFIX = 'src-type-';

export class SourceBtn extends UploaderBlock {
couldBeCtxOwner = true;
/** @private */
_registeredTypes = {};

Expand Down
23 changes: 16 additions & 7 deletions blocks/UploadCtxProvider/EventEmitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ export class EventEmitter {
*/
_timeoutStore = new Map();

/** @type {Set<import('../../abstract/Block.js').Block>} */
_targets = new Set();

/** @param {() => string} getCtxName */
constructor(getCtxName) {
/** @private */
Expand All @@ -73,8 +76,12 @@ export class EventEmitter {

/** @param {import('../../abstract/Block.js').Block} target */
bindTarget(target) {
/** @private */
this._target = target;
this._targets.add(target);
}

/** @param {import('../../abstract/Block.js').Block} target */
unbindTarget(target) {
this._targets.delete(target);
}

/**
Expand All @@ -84,11 +91,13 @@ export class EventEmitter {
* @param {EventPayload[T]} [payload]
*/
_dispatch(type, payload) {
this._target?.dispatchEvent(
new CustomEvent(type, {
detail: payload,
})
);
for (const target of this._targets) {
target.dispatchEvent(
new CustomEvent(type, {
detail: payload,
})
);
}

const globalEventType = GlobalEventType[type];
window.dispatchEvent(
Expand Down
6 changes: 6 additions & 0 deletions blocks/UploadCtxProvider/UploadCtxProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ class UploadCtxProviderClass extends UploaderBlock {

this.$['*eventEmitter'].bindTarget(this);
}

destroyCallback() {
super.destroyCallback();

this.$['*eventEmitter'].unbindTarget(this);
}
}

/**
Expand Down
3 changes: 3 additions & 0 deletions blocks/UploadList/UploadList.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import { debounce } from '../utils/debounce.js';
*/

export class UploadList extends UploaderBlock {
// Context owner should have access to CSS l10n
// TODO: We need to move away l10n from CSS
couldBeCtxOwner = true;
historyTracked = true;
activityType = ActivityBlock.activities.UPLOAD_LIST;

Expand Down
1 change: 1 addition & 0 deletions blocks/UrlSource/UrlSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ActivityBlock } from '../../abstract/ActivityBlock.js';
import { UploadSource } from '../utils/UploadSource.js';

export class UrlSource extends UploaderBlock {
couldBeCtxOwner = true;
activityType = ActivityBlock.activities.URL;

init$ = {
Expand Down
Loading