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(core/component/interface): enhance destructor options #1313

Merged
merged 4 commits into from
Jun 24, 2024
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
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;
}
Loading