Skip to content

Commit

Permalink
fix(performance): resolve a memory leak in defineClasses with wrong…
Browse files Browse the repository at this point in the history
…/missing effectScope (#1067)
  • Loading branch information
mlmoravek authored Oct 14, 2024
1 parent 923f887 commit a1f53c2
Show file tree
Hide file tree
Showing 23 changed files with 325 additions and 246 deletions.
21 changes: 12 additions & 9 deletions packages/oruga/src/components/autocomplete/Autocomplete.vue
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,11 @@ const rootClasses = defineClasses(["rootClass", "o-acp"]);
const itemClasses = defineClasses(["itemClass", "o-acp__item"]);
const itemHoverClasses = defineClasses([
"itemHoverClass",
"o-acp__item--hover",
]);
const itemEmptyClasses = defineClasses([
"itemEmptyClass",
"o-acp__item--empty",
Expand All @@ -797,15 +802,13 @@ const itemFooterClasses = defineClasses(
["itemHoverClass", "o-acp__item--hover", null, footerHovered],
);
function itemOptionClasses(option): ClassBind[] {
const optionClasses = defineClasses([
"itemHoverClass",
"o-acp__item--hover",
null,
computed(() => toRaw(option) === toRaw(hoveredOption.value)),
]);
function itemAppliedClasses(option: T): ClassBind[] {
const hoverClasses =
toRaw(option) === toRaw(hoveredOption.value)
? itemHoverClasses.value
: [];
return [...itemClasses.value, ...optionClasses.value];
return [...itemClasses.value, ...hoverClasses];
}
// --- Expose Public Functionalities ---
Expand Down Expand Up @@ -922,7 +925,7 @@ defineExpose({ focus: setFocus, value: vmodel });
:ref="(el) => setItemRef(el, groupindex, index)"
:value="option"
:tag="itemTag"
:class="itemOptionClasses(option)"
:class="itemAppliedClasses(option)"
aria-role="option"
:aria-selected="toRaw(option) === toRaw(hoveredOption)"
:tabindex="-1"
Expand Down
42 changes: 23 additions & 19 deletions packages/oruga/src/components/carousel/Carousel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -548,24 +548,6 @@ const arrowIconNextClasses = defineClasses([
"o-car__arrow__icon-next",
]);
function indicatorItemClasses(index): ClassBind[] {
return defineClasses(
["indicatorItemClass", "o-car__indicator__item"],
[
"indicatorItemActiveClass",
"o-car__indicator__item--active",
null,
indicatorIndex.value === index,
],
[
"indicatorItemStyleClass",
"o-car__indicator__item--",
props.indicatorStyle,
!!props.indicatorStyle,
],
).value;
}
const indicatorsClasses = defineClasses(
["indicatorsClass", "o-car__indicators"],
[
Expand All @@ -583,6 +565,28 @@ const indicatorsClasses = defineClasses(
);
const indicatorClasses = defineClasses(["indicatorClass", "o-car__indicator"]);
const indicatorItemClasses = defineClasses(
["indicatorItemClass", "o-car__indicator__item"],
[
"indicatorItemStyleClass",
"o-car__indicator__item--",
props.indicatorStyle,
!!props.indicatorStyle,
],
);
const indicatorItemActiveClasses = defineClasses([
"indicatorItemActiveClass",
"o-car__indicator__item--active",
]);
function indicatorItemAppliedClasses(index: number): ClassBind[] {
const activeClasses =
indicatorIndex.value === index ? indicatorItemActiveClasses.value : [];
return [...indicatorItemClasses.value, ...activeClasses];
}
</script>

<template>
Expand Down Expand Up @@ -680,7 +684,7 @@ const indicatorClasses = defineClasses(["indicatorClass", "o-car__indicator"]);
@binding {index} index indicator index
-->
<slot :index="index" name="indicator">
<span :class="indicatorItemClasses(index)" />
<span :class="indicatorItemAppliedClasses(index)" />
</slot>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/oruga/src/components/datepicker/Datepicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ const pickerDropdownClasses = defineClasses([
]);
const boxClasses = defineClasses(["boxClass", "o-dpck__box"]);
const boxClassBind = computed(() => getActiveClasses(boxClasses.value));
const boxClassBind = computed(() => getActiveClasses(boxClasses));
// --- Expose Public Functionalities ---
Expand Down
11 changes: 10 additions & 1 deletion packages/oruga/src/components/datepicker/DatepickerMonth.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
ref,
nextTick,
watch,
effectScope,
onUnmounted,
type PropType,
type ComponentPublicInstance,
} from "vue";
Expand Down Expand Up @@ -355,6 +357,12 @@ const monthCellClasses = defineClasses(
["monthCellEventsClass", "o-dpck__month__cell--events", null, hasEvents],
);
// Registers a dispose callback on the current active effect scope.
const scope = effectScope();
// stop all scope effects
onUnmounted(() => scope.stop());
/**
* Build cellClasses for cell using validations
*/
Expand All @@ -380,7 +388,6 @@ function cellClasses(day: Date): ClassBind[] {
isTrueish(props.pickerProps.multiple),
),
],
[
"monthCellFirstSelectedClass",
"o-dpck__month__cell--first-selected",
Expand Down Expand Up @@ -466,6 +473,8 @@ function cellClasses(day: Date): ClassBind[] {
null,
!isDateSelectable(day) || props.pickerProps.disabled,
],
// pass effect scope for rectivity binding
{ scope },
);
return [...monthCellClasses.value, ...classes.value];
Expand Down
11 changes: 9 additions & 2 deletions packages/oruga/src/components/datepicker/DatepickerTableRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
watch,
nextTick,
ref,
effectScope,
onUnmounted,
type PropType,
type ComponentPublicInstance,
} from "vue";
Expand Down Expand Up @@ -226,6 +228,11 @@ function dateWithin(
return dateOne > dates[0] && dateOne < dates[1];
}
const scope = effectScope();
// stop all scope effects
onUnmounted(() => scope.stop());
/** Build cellClasses for cell using validations */
function cellClasses(day: Date): ClassBind[] {
const classes = defineClasses(
Expand Down Expand Up @@ -322,22 +329,22 @@ function cellClasses(day: Date): ClassBind[] {
null,
!isDateSelectable(day, props.month) || props.pickerProps.disabled,
],
[
"tableCellInvisibleClass",
"o-dpck__table__cell--invisible",
null,
!props.pickerProps.nearbyMonthDays &&
day.getMonth() !== props.month,
],
[
"tableCellNearbyClass",
"o-dpck__table__cell--nearby",
null,
props.pickerProps.nearbySelectableMonthDays &&
day.getMonth() !== props.month,
],
// pass effect scope for rectivity binding
{ scope },
);
return [
Expand Down
5 changes: 1 addition & 4 deletions packages/oruga/src/components/datepicker/examples/slots.vue
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<script setup lang="ts">
import { ref } from "vue";
import type { OptionsItem } from "@oruga-ui/oruga-next";
const months: OptionsItem<number>[] = [
const months = [
{ label: "January", value: 0 },
{ label: "February", value: 1 },
{ label: "March", value: 2 },
Expand All @@ -18,7 +17,6 @@ const months: OptionsItem<number>[] = [
];
const date = ref<Date | undefined>(new Date());
const month = ref<number>(date.value?.getMonth() || 0);
function selectMonth(option): void {
if (!option) return;
Expand All @@ -38,7 +36,6 @@ function selectMonth(option): void {
<template #header>
<o-field grouped>
<o-autocomplete
v-model="month"
:options="months"
root-class="grow"
open-on-focus
Expand Down
1 change: 0 additions & 1 deletion packages/oruga/src/components/field/Field.vue
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,6 @@ const innerFieldClasses = defineClasses(
null,
computed(() => props.groupMultiline),
],
["groupedClass", "o-field--grouped", null, computed(() => props.grouped)],
[
"addonsClass",
Expand Down
4 changes: 1 addition & 3 deletions packages/oruga/src/components/sidebar/Sidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,7 @@ const scrollClasses = defineClasses(["scrollClipClass", "o-clipped"]);
const noScrollClasses = defineClasses(["noScrollClass", "o-noscroll"]);
const scrollClass = computed(() =>
getActiveClasses(
props.scroll === "clip" ? scrollClasses.value : noScrollClasses.value,
),
getActiveClasses(props.scroll === "clip" ? scrollClasses : noScrollClasses),
);
// --- Expose Public Functionalities ---
Expand Down
24 changes: 20 additions & 4 deletions packages/oruga/src/components/steps/StepItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ const props = defineProps({
type: String,
default: () => getOption("steps.ariaRole", "tab"),
},
/** Sets a class to the item header */
headerClass: { type: String, default: undefined },
// class props (will not be displayed in the docs)
/** Class of the content item */
itemClass: {
Expand Down Expand Up @@ -105,15 +103,16 @@ const emits = defineEmits<{
const slots = useSlots();
const providedData = computed<StepItemComponent>(() => ({
const providedData = computed<StepItemComponent<T>>(() => ({
...props,
$slots: slots,
classes: itemClasses.value,
isTransitioning: isTransitioning.value,
activate,
deactivate,
}));
// Inject functionalities and data from the parent carousel component
// inject functionalities and data from the parent carousel component
const { parent, item } = useProviderChild<StepsComponent<T>>({
data: providedData,
});
Expand Down Expand Up @@ -167,6 +166,23 @@ function beforeLeave(): void {
// --- Computed Component Classes ---
const elementClasses = defineClasses(["itemClass", "o-steps__item"]);
const itemClasses = defineClasses(
["itemHeaderClass", "o-steps__nav-item"],
[
"itemHeaderVariantClass",
"o-steps__nav-item--",
computed(() => parent.value?.variant || props.variant),
computed(() => !!parent.value?.variant || !!props.variant),
],
["itemHeaderActiveClass", "o-steps__nav-item-active", null, isActive],
[
"itemHeaderPreviousClass",
"o-steps__nav-item-previous",
null,
computed(() => item.value.index < parent.value?.activeIndex),
],
);
</script>

<template>
Expand Down
Loading

0 comments on commit a1f53c2

Please sign in to comment.