Skip to content

fix: Improve code and simplify logic #2651

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.

Sign up for GitHub

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

Merged
liujuping merged 7 commits into from
Nov 23, 2023
Merged

fix: Improve code and simplify logic #2651

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
50137b9
fix: Improve code and simplify logic
andylili21 Nov 14, 2023
299af37
#补充单元测试
andylili21 Nov 20, 2023
34059c6
#格式化
andylili21 Nov 20, 2023
a5acc3b
#Unit Testing
andylili21 Nov 20, 2023
3bfedfb
#Adding UTs
andylili21 Nov 21, 2023
c727b41
#changeing explain
andylili21 Nov 22, 2023
2b94f21
#change case
andylili21 Nov 23, 2023
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
14 changes: 7 additions & 7 deletions packages/designer/src/document/node/node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,23 +440,23 @@ export class Node<Schema extends IPublicTypeNodeSchema = IPublicTypeNodeSchema>
}

private initialChildren(children: IPublicTypeNodeData | IPublicTypeNodeData[] | undefined): IPublicTypeNodeData[] {
// FIXME! this is dirty code
const { initialChildren } = this.componentMeta.advanced;

if (children == null) {
const { initialChildren } = this.componentMeta.advanced;
if (initialChildren) {
if (typeof initialChildren === 'function') {
return initialChildren(this.internalToShellNode()!) || [];
}
return initialChildren;
}
return [];
}

if (Array.isArray(children)) {
return children;
} else if (children) {
return [children];
} else {
return [];
}

return [children];
Copy link
Collaborator

@liujuping liujuping Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

children 为 undefined 的情况和原来输出是否一样。重构建议先补充相关的单元测试用例。

Copy link
Contributor Author

@andylili21 andylili21 Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

Copy link
Contributor Author

@andylili21 andylili21 Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

想问一下补充相关的单元测试用例，可以在node.test.ts文件中添加测试用例，来测试函数在重构前后输出结果是否一样吗？

Copy link
Collaborator

@liujuping liujuping Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以的。

}

isContainer(): boolean {
Expand Down Expand Up @@ -1094,7 +1094,7 @@ export class Node<Schema extends IPublicTypeNodeSchema = IPublicTypeNodeSchema>
}

/**
* 是否可执行某action
* 是否可执行某 action
*/
canPerformAction(actionName: string): boolean {
const availableActions =
Expand Down
64 changes: 60 additions & 4 deletions packages/designer/tests/document/node/node.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import rootFooterMetadata from '../../fixtures/component-metadata/root-footer';
import { shellModelFactory } from '../../../../engine/src/modules/shell-model-factory';
import { isNode } from '@alilc/lowcode-utils';
import { Setters } from '@alilc/lowcode-shell';
import { IPublicTypeNodeData } from '@alilc/lowcode-types';

describe('Node 方法测试', () => {
let editor: Editor;
Expand All @@ -53,8 +54,63 @@ describe('Node 方法测试', () => {
designer = null;
project = null;
});
//测试 children 为 undefined 时重构前后输出结果
it('initialChildren and initialChildren2 should return the same result when children is undefined', () => {
//重构前的 Node 的 initialChildren 方法
Copy link
Collaborator

@liujuping liujuping Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你的测试不应该自己模拟函数，应该用类似于下面的方式写。

// 导入Node类
const { Node } = require('./Node'); // 假设Node类位于Node.js文件中

// 创建一个测试套件
describe('Node Class', () => {
  // 创建一个测试用例
  it('should initialize children array with empty array when children is null', () => {
    // 创建一个Node实例
    const node = new Node();

    // 调用initialChildren方法并传入null作为参数
    const result = node.initialChildren(null);

    // 预期结果是一个空数组
    expect(result).to.deep.equal([]);
  });

  // 创建更多的测试用例来测试其他情况...
});

Copy link
Contributor Author

@andylili21 andylili21 Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK 问一下怎么比较改进前的方法🤔️

Copy link
Collaborator

@liujuping liujuping Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不需要比较改进前的方法，主要看改进后的输入输出。

Copy link
Contributor Author

@andylili21 andylili21 Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1葫画瓢 这样画的可以嘛

Copy link
Collaborator

@liujuping liujuping Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以的，可以再加一下 0 和 false 的测试用例。

Copy link
Contributor Author

@andylili21 andylili21 Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

function initialChildren(children: IPublicTypeNodeData | IPublicTypeNodeData[] | undefined): IPublicTypeNodeData[] {
// FIXME! this is dirty code
if (children == null) {
const { initialChildren } = {
callbacks: () => { },
getResizingHandlers: () => { },
}
if (initialChildren) {
if (typeof initialChildren === 'function') {
return [];
}
return initialChildren;
}
}
if (Array.isArray(children)) {
return children;
} else if (children) {
return [children];
} else {
return [];
}
}

//重构后的 Node 的 initialChildren 方法
function initialChildren2(children: IPublicTypeNodeData | IPublicTypeNodeData[] | undefined): IPublicTypeNodeData[] {
const { initialChildren } = {
callbacks: () => { },
getResizingHandlers: () => { },
}

if (children == null) {
if (initialChildren) {
if (typeof initialChildren === 'function') {
return [];
}
return initialChildren;
}
return [];
}

if (Array.isArray(children)) {
return children;
}

return [children];
}
const children: IPublicTypeNodeData | IPublicTypeNodeData[] | undefined = undefined;
const result1 = initialChildren(children);
const result2 = initialChildren2(children);

expect(result1).toEqual(result2);
});

it('condition group', () => {});
it('condition group', () => { });

it('getExtraProp / setExtraProp', () => {
const firstBtn = doc.getNode('node_k1ow3cbn')!;
Expand Down Expand Up @@ -367,7 +423,7 @@ describe('Node 方法测试', () => {
expect(mockFn).not.toHaveBeenCalled();
});

it('addSlot / unlinkSlot / removeSlot', () => {});
it('addSlot / unlinkSlot / removeSlot', () => { });

it('setProps', () => {
const firstBtn = doc.getNode('node_k1ow3cbn')!;
Expand Down Expand Up @@ -407,7 +463,7 @@ describe('Node 方法测试', () => {
designer.createComponentMeta(btnMetadata);
const btn = doc.getNode('node_k1ow3cbn');
// 从 componentMeta 中获取到 title 值
expect(btn.title).toEqual({ type: 'i18n', 'zh-CN': '按钮', 'en-US': 'Button' } );
expect(btn.title).toEqual({ type: 'i18n', 'zh-CN': '按钮', 'en-US': 'Button' });
// 从 extraProp 中获取值
btn.setExtraProp('title', 'hello button');
expect(btn.title).toBe('hello button');
Expand Down Expand Up @@ -546,7 +602,7 @@ describe('Node 方法测试', () => {
expect(comparePosition(firstBtn, firstCard)).toBe(PositionNO.BeforeOrAfter);
});

it('getZLevelTop', () => {});
it('getZLevelTop', () => { });
it('propsData', () => {
expect(new Node(doc, { componentName: 'Leaf' }).propsData).toBeNull();
expect(new Node(doc, { componentName: 'Fragment' }).propsData).toBeNull();
Expand Down