Skip to content

Commit

Permalink
fix(core/component/interface): enhance destructor options (#1313)
Browse files Browse the repository at this point in the history
* fix(core/component/interface): enhance destructor options

Enable control over both the recursion of the destructor and the execution of unmount handlers in the directives used within the component

* docs: update changelog

* chore(core/component/interface): rename callUnmount in destructor opts

* tests(core/component/functional): test vnode unmount is not called on re-creation
  • Loading branch information
shining-mind authored Jun 24, 2024
1 parent 833a9ea commit 647da2f
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 34 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ Changelog
_Note: Gaps between patch versions are faulty, broken or test releases._

## v4.0.0-beta.?? (2024-06-??)

#### :bug: Bug Fix

* Fixed unwanted execution of unmount handlers in the directives used
within the functional component during its re-creation.
The `$destroy` method now accepts an object with options, which enables control over
both the recursion of the destructor and the unmounting of vnodes
within the component `core/component/interface`

## v4.0.0-beta.104 (2024-06-19)

#### :rocket: New Feature
Expand Down
2 changes: 1 addition & 1 deletion src/components/super/i-block/test/unit/destructor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ test.describe('<i-block> calling a component\'s destructor', () => {
dynamicChild = await target.evaluateHandle<iBlock>((ctx) => ctx.$refs.child?.[1]);

await dynamicChild.evaluate((ctx) => {
ctx.unsafe.$destroy(false);
ctx.unsafe.$destroy({recursive: false});
});

// Wait until memory is cleaned up
Expand Down
7 changes: 5 additions & 2 deletions src/core/component/engines/directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { ComponentEngine } from 'core/component/engines/engine';
import type { Directive, DirectiveBinding, VNode } from 'core/component/engines/interface';
import type { ComponentDestructorOptions } from 'core/component/interface';

// eslint-disable-next-line @v4fire/unbound-method
const staticDirective = ComponentEngine.directive.length > 0 ? ComponentEngine.directive : null;
Expand Down Expand Up @@ -63,8 +64,10 @@ ComponentEngine.directive = function directive(name: string, directive?: Directi
}

if (vnode.virtualContext != null) {
vnode.virtualContext.unsafe.$once('[[BEFORE_DESTROY]]', () => {
originalUnmounted.apply(this, args);
vnode.virtualContext.unsafe.$once('[[BEFORE_DESTROY]]', (opts: Required<ComponentDestructorOptions>) => {
if (opts.shouldUnmountVNodes) {
originalUnmounted.apply(this, args);
}
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/component/engines/vue3/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export function getComponent(meta: ComponentMeta): ComponentOptions<typeof Compo
},

beforeUnmount(): void {
init.beforeDestroyState(getComponentContext(this), false);
init.beforeDestroyState(getComponentContext(this), {recursive: false});
},

unmounted(): void {
Expand Down
5 changes: 3 additions & 2 deletions src/core/component/functional/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import * as init from 'core/component/init';
import type { ComponentInterface, ComponentElement } from 'core/component/interface';
import type { ComponentInterface, ComponentElement, ComponentDestructorOptions } from 'core/component/interface';

/**
* Initializes the default dynamic lifecycle handlers for the given functional component.
Expand Down Expand Up @@ -76,7 +76,8 @@ export function inheritContext(

// Here, the functional component is recreated during re-rendering.
// Therefore, the destructor call should not recursively propagate to child components.
parentCtx.unsafe.$destroy(false);
// Also, we should not unmount the vnodes created within the component.
parentCtx.unsafe.$destroy(<ComponentDestructorOptions>{recursive: false, shouldUnmountVNodes: false});

const
props = ctx.$props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@
}, attrs) .

- block body
Click!
< . v-hook = {unmounted: reset}
Click!
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,9 @@ export default class bFunctionalButtonDummy extends bDummy {
this.clickCount += 1;
this.uniqueClickCount += 1;
}

reset(): void {
this.clickCount = 0;
this.uniqueClickCount = 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default class bFunctionalDummy extends bDummy {
button: bFunctionalButtonDummy;
};

updateClickCount(): void {
updateClickCountField(): void {
this.clickCount = this.clickCountStore;
}

Expand Down
35 changes: 25 additions & 10 deletions src/core/component/functional/test/unit/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,44 @@ test.describe('functional component', () => {
await clickAndWaitForEvent();
await test.expect(text).toHaveText('Click count: 0');

await target.evaluate((ctx) => ctx.updateClickCount());
await target.evaluate((ctx) => ctx.updateClickCountField());
await test.expect(text).toHaveText('Click count: 1');

await clickAndWaitForEvent();
});

test('should reset counters on vnode unmount', async () => {
await test.expect(clickAndGetCounts()).resolves.toEqual([1, 1]);

const clicks = await target.evaluate((ctx) => {
const {button} = ctx.unsafe.$refs;
button.unsafe.$destroy();

const {clickCount, uniqueClickCount} = button;
return [clickCount, uniqueClickCount];
});

test.expect(clicks).toEqual([0, 0]);
});

test([
'should handle system properties correctly:',
'reset the unique ones and keep the regular ones'
].join(' '), async () => {
const clickAndGetCounts = async () => {
await button.click();
return target.evaluate((ctx) => {
const {clickCount, uniqueClickCount} = ctx.unsafe.$refs.button;
return [clickCount, uniqueClickCount];
});
};

await test.expect(clickAndGetCounts()).resolves.toEqual([1, 1]);
await test.expect(clickAndGetCounts()).resolves.toEqual([2, 2]);

await target.evaluate((ctx) => ctx.updateClickCount());
await target.evaluate((ctx) => ctx.updateClickCountField());
await test.expect(clickAndGetCounts()).resolves.toEqual([3, 1]);
});

async function clickAndGetCounts() {
await button.click();
return target.evaluate((ctx) => {
const {clickCount, uniqueClickCount} = ctx.unsafe.$refs.button;
return [clickCount, uniqueClickCount];
});
}

});
});
6 changes: 3 additions & 3 deletions src/core/component/init/states/before-create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { implementEventEmitterAPI } from 'core/component/event';
import { beforeDestroyState } from 'core/component/init/states/before-destroy';
import { destroyedState } from 'core/component/init/states/destroyed';

import type { ComponentInterface, ComponentMeta, ComponentElement } from 'core/component/interface';
import type { ComponentInterface, ComponentMeta, ComponentElement, ComponentDestructorOptions } from 'core/component/interface';
import type { InitBeforeCreateStateOptions } from 'core/component/init/interface';

/**
Expand Down Expand Up @@ -65,8 +65,8 @@ export function beforeCreateState(
configurable: true,
enumerable: false,
writable: true,
value: (recursive: boolean = true) => {
beforeDestroyState(component, recursive);
value: (opts: ComponentDestructorOptions) => {
beforeDestroyState(component, opts);
destroyedState(component);
}
});
Expand Down
12 changes: 7 additions & 5 deletions src/core/component/init/states/before-destroy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@ import { callMethodFromComponent } from 'core/component/method';
import { runHook } from 'core/component/hook';
import { destroyedHooks } from 'core/component/const';

import type { ComponentInterface } from 'core/component/interface';
import type { ComponentInterface, ComponentDestructorOptions } from 'core/component/interface';

/**
* Initializes the "beforeDestroy" state to the specified component instance
*
* @param component
* @param [recursive] - if set to false, the destructor will be executed for the component itself,
* but not for its descendants
* @param [opts]
*/
export function beforeDestroyState(component: ComponentInterface, recursive: boolean = true): void {
export function beforeDestroyState(component: ComponentInterface, opts: ComponentDestructorOptions = {}): void {
if (destroyedHooks[component.hook] != null) {
return;
}
Expand All @@ -33,7 +32,10 @@ export function beforeDestroyState(component: ComponentInterface, recursive: boo
unsafe: {$el}
} = component;

unsafe.$emit('[[BEFORE_DESTROY]]', recursive);
unsafe.$emit('[[BEFORE_DESTROY]]', <Required<ComponentDestructorOptions>>{
recursive: opts.recursive ?? true,
shouldUnmountVNodes: opts.shouldUnmountVNodes ?? true
});

unsafe.async.clearAll().locked = true;
unsafe.$async.clearAll().locked = true;
Expand Down
8 changes: 4 additions & 4 deletions src/core/component/init/states/created.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { destroyedHooks } from 'core/component/const';
import { callMethodFromComponent } from 'core/component/method';
import { runHook } from 'core/component/hook';

import type { ComponentInterface, Hook } from 'core/component/interface';
import type { ComponentDestructorOptions, ComponentInterface, Hook } from 'core/component/interface';

const
remoteActivationLabel = Symbol('The remote activation label');
Expand Down Expand Up @@ -43,14 +43,14 @@ export function createdState(component: ComponentInterface): void {
isRegularComponent = unsafe.meta.params.functional !== true,
isDynamicallyMountedComponent = '$remoteParent' in r;

const destroy = (recursive: boolean) => {
const destroy = (opts: Required<ComponentDestructorOptions>) => {
// A component might have already been removed by explicitly calling $destroy
if (destroyedHooks[unsafe.hook] != null) {
return;
}

if (recursive || isDynamicallyMountedComponent) {
unsafe.$destroy(recursive);
if (opts.recursive || isDynamicallyMountedComponent) {
unsafe.$destroy(opts);
}
};

Expand Down
11 changes: 11 additions & 0 deletions src/core/component/interface/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ Changelog
> - :house: [Internal]
> - :nail_care: [Polish]
## v4.0.0-beta.?? (2024-06-??)

#### :bug: Bug Fix

* Fixed unwanted execution of unmount handlers in the directives used
within the functional component during its re-creation.
The `$destroy` method now accepts an object with options, which enables control over
both the recursion of the destructor and the unmounting of vnodes
within the component


## v4.0.0-beta.104 (2024-06-19)

#### :rocket: New Feature
Expand Down
7 changes: 3 additions & 4 deletions src/core/component/interface/component/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type { ModsProp, ModsDict } from 'core/component/interface/mod';
import type { SyncLinkCache } from 'core/component/interface/link';
import type { RenderEngine } from 'core/component/interface/engine';

import type { ComponentElement, ComponentEmitterOptions } from 'core/component/interface/component/types';
import type { ComponentDestructorOptions, ComponentElement, ComponentEmitterOptions } from 'core/component/interface/component/types';
import type { WatchPath, WatchOptions, RawWatchHandler } from 'core/component/interface/watch';
import type { UnsafeGetter, UnsafeComponentInterface } from 'core/component/interface/component/unsafe';

Expand Down Expand Up @@ -374,10 +374,9 @@ export abstract class ComponentInterface {

/**
* Destroys the component
* @param [_recursive] - if set to false, the destructor will be executed for the component itself,
* but not for its descendants
* @param [_opts]
*/
protected $destroy(_recursive: boolean = true): void {
protected $destroy(_opts?: ComponentDestructorOptions): void {
return Object.throw();
}

Expand Down
12 changes: 12 additions & 0 deletions src/core/component/interface/component/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,15 @@ export interface ComponentEmitterOptions {
*/
rawEmitter?: boolean;
}

export interface ComponentDestructorOptions {
/**
* If set to false, the destructor will be executed for the component itself, but not for its descendants
*/
recursive?: boolean;

/**
* If set to false, the vnodes won't be unmounted within the component
*/
shouldUnmountVNodes?: boolean;
}

0 comments on commit 647da2f

Please sign in to comment.