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

refactor: runtime structure #63

Merged
merged 28 commits into from
May 11, 2022
Merged

refactor: runtime structure #63

merged 28 commits into from
May 11, 2022

Conversation

SoloJiang
Copy link
Contributor

SoloJiang and others added 9 commits April 25, 2022 10:12
* feat: support svg elements (#60)

* feat: support svg elements

* chore: fix lint and add notes

* feat(compiler): update structure in get template method

* fix(compiler): lint error

* feat(compiler): use handler to distinguish events and on props

* chore(compiler): add EventAttributeDescriptor interface

Co-authored-by: Rongyan Chen <[email protected]>
Co-authored-by: 狒狒神 <[email protected]>
@cryzzchen
Copy link

@SoloJiang 没有考虑数组中存在非模板节点的情况,TEMPLATES 命名应当为 MULTI 或其他单纯表示数组的名称

this.#reactiveNodes[index].commitValue(values[index]);
}
}
const currentElementTemplate = this.template;

Choose a reason for hiding this comment

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

组件的整个模板结构应当为一个 templateNode,在这里 commitValue,把 updateView 的逻辑移到 templateNode 中去

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,我也这么想的,有一些细节还要处理下

@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #63 (87202f1) into main (e5b302d) will increase coverage by 1.72%.
The diff coverage is 93.40%.

@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   85.57%   87.29%   +1.72%     
==========================================
  Files          41       45       +4     
  Lines         804      921     +117     
  Branches      182      204      +22     
==========================================
+ Hits          688      804     +116     
- Misses        116      117       +1     
Impacted Files Coverage Δ
...kages/pwc-compiler/src/compileTemplateInRuntime.ts 0.00% <0.00%> (ø)
packages/pwc/src/elements/renderElementTemplate.ts 86.95% <86.95%> (ø)
packages/pwc/src/elements/reactiveNode.ts 97.64% <97.33%> (-2.36%) ⬇️
packages/pwc-compiler/src/compileScript.ts 100.00% <100.00%> (ø)
packages/pwc-compiler/src/compileTemplate.ts 95.77% <100.00%> (ø)
...ages/pwc-compiler/src/transform/autoAddAccessor.ts 100.00% <100.00%> (ø)
...pwc-compiler/src/transform/genGetTemplateMethod.ts 100.00% <100.00%> (ø)
packages/pwc-compiler/src/transform/index.ts 100.00% <100.00%> (ø)
packages/pwc/src/constants.ts 100.00% <100.00%> (ø)
packages/pwc/src/decorators/attribute/index.ts 81.57% <100.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5b302d...87202f1. Read the comment docs.

import { ElementTemplate, PWCElementTemplate } from '../type';
import { elementTemplateManager } from './elementTemplateManager';

export function getTemplateInfo(elementTemplate: ElementTemplate): PWCElementTemplate {

Choose a reason for hiding this comment

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

这个函数名要不改成 formatTemplateInfo ?

const commentNode = document.createComment(TEXT_COMMENT_DATA);
this.appendChild(commentNode);
if (isArray(this.#currentTemplate)) {
this.#reactiveNodes.push(

Choose a reason for hiding this comment

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

reactiveNode 应该只有一个了,不需要 push 到一个数组里面?

}
}

function deleteChildren(targetReactiveNode: ReactiveNode) {

Choose a reason for hiding this comment

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

这里的 deleteChildren 其实现与 TemplatesNode 耦合,且不存在其他脱离 TemplatesNode 调用的情况,因此放在 TemplatesNode 中是否更好?OO思想

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯,原本是这么设计的,只是希望可以针对方法补充测试用例,现在来看这个方法比较简单似乎没有必要

targetReactiveNode.reactiveNodes = [];
}

export function updateView(

Choose a reason for hiding this comment

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

这个也是

@@ -31,3 +33,11 @@ export function isMap(value: unknown) {
export function isPrivate(name: string) {
return name.startsWith('#');
}

export function isTemplate(value: unknown): boolean {
return value && value[TemplateFlag] === true;

Choose a reason for hiding this comment

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

我记得当时的讨论结果是,同时判断三个属性

import { throwError } from '../error';
import { elementTemplateManager } from './elementTemplateManager';

export function initRenderTemplate(

Choose a reason for hiding this comment

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

增加注释

}
this.#currentTemplate = this.template;
this.#root = this.shadowRoot || this;
const commentNode = document.createComment(TEXT_COMMENT_DATA);

Choose a reason for hiding this comment

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

这个位置加一个注释,说明一下根位置增加一个注释节点的原因吧

// @ts-ignore
this.__init_task__();
}
this.#currentTemplate = this.template;

Choose a reason for hiding this comment

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

this.template format 之后赋值给 this.#currentTemplate,删除 44 行

@SoloJiang SoloJiang merged commit 6eddb4f into main May 11, 2022
@delete-merged-branch delete-merged-branch bot deleted the refactor/runtime-structure branch May 11, 2022 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants