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: supportVariable SHOULD take precedence over supportVariableGlobally #1997

Merged
merged 2 commits into from May 29, 2023

Conversation

LeoYuan
Copy link
Contributor

@LeoYuan LeoYuan commented May 8, 2023

Refs: #177 #500 #1938 #1930

Summary: supportVariable in single prop definition should OVERLAP the global valve, a.k.a. supportVariableGlobally~

For Example:

  1. supportVariable: false + supportVariableGlobally: true
    -> false
  2. supportVariable: true | undefined + supportVariableGlobally: true
    -> true
  3. supportVariable: true | undefined + supportVariableGlobally: false
    -> true

@github-actions
Copy link

github-actions bot commented May 8, 2023

ChatGPT Code Review:

代码的功能是:提供一个函数 shouldUseVariableSetter,这个函数有两个参数 propSupportVariable 和 globalSupportVariable,分别表示组件定义中的支持变量属性和全局支持变量属性。这个函数的作用是判断 propSupportVariable 和 globalSupportVariable 的值,根据规则返回一个布尔值,表示是否应该使用 VariableSetter。函数的判断规则是:如果 propSupportVariable 是 false,就不使用 VariableSetter;否则,如果 propSupportVariable 是 true 或 undefined,就使用 VariableSetter,而且 propSupportVariable 优先于 globalSupportVariable。函数的测试用例通过了。另外,代码还对应修改了两个文件,修复了一个 bug 和改进了一个特性。

@github-actions
Copy link

github-actions bot commented May 8, 2023

Coverage report for packages/utils

St.
Category Percentage Covered / Total
🔴 Statements
48.8% (+0.62% 🔼)
122/250
🔴 Branches
43.49% (+0.85% 🔼)
117/269
🔴 Functions
41.77% (+0.75% 🔼)
33/79
🔴 Lines
49.79% (+0.43% 🔼)
117/235

Test suite run success

33 tests passing in 6 suites.

Report generated by 🧪jest coverage report action from db03e98

@github-actions
Copy link

github-actions bot commented May 8, 2023

Coverage report for packages/react-simulator-renderer

St.
Category Percentage Covered / Total
🔴 Statements 33.63% 114/339
🔴 Branches 14.88% 25/168
🔴 Functions 26.14% 23/88
🔴 Lines 34.23% 114/333

Test suite run success

2 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from db03e98

@github-actions
Copy link

github-actions bot commented May 8, 2023

Coverage report for packages/renderer-core

St.
Category Percentage Covered / Total
🟡 Statements 71.95% 908/1262
🟡 Branches 61.23% 567/926
🟡 Functions 69.12% 197/285
🟡 Lines 71.94% 892/1240

Test suite run success

91 tests passing in 8 suites.

Report generated by 🧪jest coverage report action from db03e98

@github-actions
Copy link

github-actions bot commented May 8, 2023

Coverage report for packages/designer

St.
Category Percentage Covered / Total
🟢 Statements 96.41% 2843/2949
🟢 Branches
89.05% (-0.01% 🔻)
1626/1826
🟢 Functions 95.97% 858/894
🟢 Lines 96.48% 2770/2871
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢 project/project.ts 96.8%
90.67% (-0.24% 🔻)
97.83% 96.52%

Test suite run success

370 tests passing in 42 suites.

Report generated by 🧪jest coverage report action from db03e98

@liujuping
Copy link
Collaborator

1.supportVariable: false + supportVariableGlobally: true
-> false
2.supportVariable: true | undefined + supportVariableGlobally: true
-> true
3.supportVariable: true | undefined + supportVariableGlobally: false
-> true

这几个示例看看可以补充一下对应的单测吗?

另外,supportVariable:undefined + supportVariableGlobally: false 是不是应该是 false。

@LeoYuan
Copy link
Contributor Author

LeoYuan commented May 8, 2023

哈哈,被你发现了,第三个写错了~

简单来说,就是supportVariable为false形成了一个短路逻辑,其他不变。

单测我觉得可以不写,比较简单,不然还得重构一个shouldUseVar的函数,然后单测这个函数?

@liujuping
Copy link
Collaborator

supportVariable: true + supportVariableGlobally: false => true

这个看逻辑好像是 false?我理解应该是 true。单测要不还是加上吧,用一个单独的函数可行,主要是这里的有多个情况,后面不一定会验证到,有单测有保证一点。

@liujuping liujuping merged commit 503793f into develop May 29, 2023
18 checks passed
@liujuping liujuping deleted the fix/support-variable branch May 29, 2023 02:12
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.

None yet

2 participants