44r0n7/privacy.sexy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix memory leaks via auto-unsubscribing and DI

Browse Source 
This commit simplifies event handling, providing a unified and robust
way to handle event lifecycling. This way, it fixes events not being
unsubscribed when state is changed.

Introduce a new function in `EventSubscriptionCollection` to remove
existing events and adding new events. This provides an easier to use
API, which leads to code that's easier to understand. It also prevents
potential bugs that may occur due to forgetting to call both functions.
It fixes `TheScriptsMenu` not unregistering events on state change.
Other improvements include:
  - Include a getter to get total amount of registered subcriptions.
    This helps in unit testing.
  - Have nullish checks to prevent potential errors further down the
    execution.
  - Use array instead of rest parameters to increase readability and
    simplify tests.

Ensure `SliderHandler` stops resizes on unmount, unsubscribing from all
events and resetting state to default.

Update `injectionKeys` to do imports as types to avoid circular
dependencies. Simplify importing `injectionKeys` to enable and strict
typings for iterating injection keys.

Add tests covering new behavior.
This commit is contained in:
undergroundwires
2023-09-01 18:14:25 +02:00
parent 19e42c9c52
commit eb096d07e2
37 changed files with 856 additions and 243 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/presentation/components/Scripts/View/Cards/CardList.vue
View File

@@ -37,7 +37,7 @@ import {
defineComponent, ref, onMounted, onUnmounted, computed,
inject,
} from 'vue';
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { InjectionKeys } from '@/presentation/injectionSymbols';
import SizeObserver from '@/presentation/components/Shared/SizeObserver.vue';
import { hasDirective } from './NonCollapsingDirective';
import CardListItem from './CardListItem.vue';
@@ -48,7 +48,7 @@ export default defineComponent({
SizeObserver,
},
setup() {
const { currentState, onStateChange } = inject(useCollectionStateKey)();
const { currentState, onStateChange } = inject(InjectionKeys.useCollectionState)();
const width = ref<number>(0);
const categoryIds = computed<ReadonlyArray<number>>(() => currentState

20
src/presentation/components/Scripts/View/Cards/CardListItem.vue
View File

@@ -52,7 +52,7 @@ import {
defineComponent, ref, watch, computed,
inject,
} from 'vue';
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { InjectionKeys } from '@/presentation/injectionSymbols';
import ScriptsTree from '@/presentation/components/Scripts/View/ScriptsTree/ScriptsTree.vue';
import { sleep } from '@/infrastructure/Threading/AsyncSleep';
@@ -76,7 +76,8 @@ export default defineComponent({
/* eslint-enable @typescript-eslint/no-unused-vars */
},
setup(props, { emit }) {
const { events, onStateChange, currentState } = inject(useCollectionStateKey)();
const { onStateChange, currentState } = inject(InjectionKeys.useCollectionState)();
const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
const isExpanded = computed({
get: () => {
@@ -106,12 +107,13 @@ export default defineComponent({
isExpanded.value = false;
}
onStateChange(async (state) => {
events.unsubscribeAll();
events.register(state.selection.changed.on(
() => updateSelectionIndicators(props.categoryId),
));
await updateSelectionIndicators(props.categoryId);
onStateChange((state) => {
events.unsubscribeAllAndRegister([
state.selection.changed.on(
() => updateSelectionIndicators(props.categoryId),
),
]);
updateSelectionIndicators(props.categoryId);
}, { immediate: true });
watch(
@@ -124,7 +126,7 @@ export default defineComponent({
cardElement.value.scrollIntoView({ behavior: 'smooth' });
}
async function updateSelectionIndicators(categoryId: number) {
function updateSelectionIndicators(categoryId: number) {
const category = currentState.value.collection.findCategory(categoryId);
const { selection } = currentState.value;
isAnyChildSelected.value = category ? selection.isAnySelected(category) : false;

23
src/presentation/components/Scripts/View/ScriptsTree/ScriptsTree.vue
View File

@@ -17,12 +17,13 @@
import {
defineComponent, watch, ref, inject,
} from 'vue';
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { InjectionKeys } from '@/presentation/injectionSymbols';
import { IScript } from '@/domain/IScript';
import { ICategory } from '@/domain/ICategory';
import { ICategoryCollectionState, IReadOnlyCategoryCollectionState } from '@/application/Context/State/ICategoryCollectionState';
import { IFilterResult } from '@/application/Context/State/Filter/IFilterResult';
import { SelectedScript } from '@/application/Context/State/Selection/SelectedScript';
import { IEventSubscription } from '@/infrastructure/Events/IEventSource';
import {
parseAllCategories, parseSingleCategory, getScriptNodeId, getCategoryNodeId, getCategoryId,
getScriptId,
@@ -43,8 +44,9 @@ export default defineComponent({
},
setup(props) {
const {
modifyCurrentState, currentState, onStateChange, events,
} = inject(useCollectionStateKey)();
modifyCurrentState, currentState, onStateChange,
} = inject(InjectionKeys.useCollectionState)();
const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
const nodes = ref<ReadonlyArray<INodeContent>>([]);
const selectedNodeIds = ref<ReadonlyArray<string>>([]);
@@ -54,7 +56,7 @@ export default defineComponent({
watch(
() => props.categoryId,
async (newCategoryId) => { await setNodes(newCategoryId); },
(newCategoryId) => setNodes(newCategoryId),
{ immediate: true },
);
@@ -63,8 +65,7 @@ export default defineComponent({
if (!props.categoryId) {
nodes.value = parseAllCategories(state.collection);
}
events.unsubscribeAll();
subscribeToState(state);
events.unsubscribeAllAndRegister(subscribeToState(state));
}, { immediate: true });
function toggleNodeSelection(event: INodeSelectedEvent) {
@@ -87,7 +88,7 @@ export default defineComponent({
|| containsCategory(node, filtered.categoryMatches);
}
async function setNodes(categoryId?: number) {
function setNodes(categoryId?: number) {
if (categoryId) {
nodes.value = parseSingleCategory(categoryId, currentState.value.collection);
} else {
@@ -97,8 +98,10 @@ export default defineComponent({
.map((selected) => getScriptNodeId(selected.script));
}
function subscribeToState(state: IReadOnlyCategoryCollectionState) {
events.register(
function subscribeToState(
state: IReadOnlyCategoryCollectionState,
): IEventSubscription[] {
return [
state.selection.changed.on((scripts) => handleSelectionChanged(scripts)),
state.filter.filterChanged.on((event) => {
event.visit({
@@ -111,7 +114,7 @@ export default defineComponent({
},
});
}),
);
];
}
function setCurrentFilter(currentFilter: IFilterResult | undefined) {

18
src/presentation/components/Scripts/View/ScriptsTree/SelectableTree/Node/RevertToggle.vue
View File

@@ -12,7 +12,7 @@ import {
computed, inject,
} from 'vue';
import { SelectedScript } from '@/application/Context/State/Selection/SelectedScript';
import { useCollectionStateKey } from '@/presentation/injectionSymbols';
import { InjectionKeys } from '@/presentation/injectionSymbols';
import { IReverter } from './Reverter/IReverter';
import { INodeContent } from './INodeContent';
import { getReverter } from './Reverter/ReverterFactory';
@@ -30,8 +30,9 @@ export default defineComponent({
},
setup(props) {
const {
currentState, modifyCurrentState, onStateChange, events,
} = inject(useCollectionStateKey)();
currentState, modifyCurrentState, onStateChange,
} = inject(InjectionKeys.useCollectionState)();
const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
const isReverted = ref(false);
@@ -39,24 +40,23 @@ export default defineComponent({
watch(
() => props.node,
async (node) => { await onNodeChanged(node); },
(node) => onNodeChanged(node),
{ immediate: true },
);
onStateChange((newState) => {
updateRevertStatusFromState(newState.selection.selectedScripts);
events.unsubscribeAll();
events.register(
events.unsubscribeAllAndRegister([
newState.selection.changed.on((scripts) => updateRevertStatusFromState(scripts)),
);
]);
}, { immediate: true });
async function onNodeChanged(node: INodeContent) {
function onNodeChanged(node: INodeContent) {
handler = getReverter(node, currentState.value.collection);
updateRevertStatusFromState(currentState.value.selection.selectedScripts);
}
async function updateRevertStatusFromState(scripts: ReadonlyArray<SelectedScript>) {
function updateRevertStatusFromState(scripts: ReadonlyArray<SelectedScript>) {
isReverted.value = handler?.getState(scripts) ?? false;
}

36
src/presentation/components/Scripts/View/TheScriptsView.vue
View File

@@ -41,7 +41,7 @@ import {
defineComponent, PropType, ref, computed,
inject,
} from 'vue';
import { useApplicationKey, useCollectionStateKey } from '@/presentation/injectionSymbols';
import { InjectionKeys } from '@/presentation/injectionSymbols';
import ScriptsTree from '@/presentation/components/Scripts/View/ScriptsTree/ScriptsTree.vue';
import CardList from '@/presentation/components/Scripts/View/Cards/CardList.vue';
import { ViewType } from '@/presentation/components/Scripts/Menu/View/ViewType';
@@ -60,8 +60,9 @@ export default defineComponent({
},
},
setup() {
const { modifyCurrentState, onStateChange, events } = inject(useCollectionStateKey)();
const { info } = inject(useApplicationKey);
const { modifyCurrentState, onStateChange } = inject(InjectionKeys.useCollectionState)();
const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
const { info } = inject(InjectionKeys.useApplication);
const repositoryUrl = computed<string>(() => info.repositoryWebUrl);
const searchQuery = ref<string | undefined>();
@@ -77,9 +78,10 @@ export default defineComponent({
});
onStateChange((newState) => {
events.unsubscribeAll();
updateFromInitialFilter(newState.filter.currentFilter);
subscribeToFilterChanges(newState.filter);
events.unsubscribeAllAndRegister([
subscribeToFilterChanges(newState.filter),
]);
}, { immediate: true });
function clearSearchQuery() {
@@ -95,19 +97,17 @@ export default defineComponent({
}
function subscribeToFilterChanges(filter: IReadOnlyUserFilter) {
events.register(
filter.filterChanged.on((event) => {
event.visit({
onApply: (newFilter) => {
searchQuery.value = newFilter.query;
searchHasMatches.value = newFilter.hasAnyMatches();
},
onClear: () => {
searchQuery.value = undefined;
},
});
}),
);
return filter.filterChanged.on((event) => {
event.visit({
onApply: (newFilter) => {
searchQuery.value = newFilter.query;
searchHasMatches.value = newFilter.hasAnyMatches();
},
onClear: () => {
searchQuery.value = undefined;
},
});
});
}
return {