-
Notifications
You must be signed in to change notification settings - Fork 326
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
feat: tinyvue grid slot generate code to template #756
base: refactor/develop
Are you sure you want to change the base?
feat: tinyvue grid slot generate code to template #756
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/vue-generator/.eslintrc.cjs
WalkthroughThis pull request introduces modifications to the Vue generator's component generation process, focusing on enhancing the handling of TinyGrid components and their columns. The changes span multiple files, including generator configurations, template generation, and test case updates. The primary modifications involve adjusting hooks, transforming slot and column handling, and updating component mappings to support more flexible grid column definitions. Changes
Sequence DiagramsequenceDiagram
participant Generator
participant Template
participant ComponentHooks
participant Columns
Generator->>ComponentHooks: Apply handleTinyGrid hook
ComponentHooks->>Template: Transform slots
Template->>Columns: Process column definitions
Columns-->>Template: Return transformed columns
Template-->>Generator: Generate component with new structure
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
7ee5602
to
1ba0e01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (4)
70-89
: Robustness intransformSlots
functionThe
transformSlots
function effectively transforms slot configurations. However, consider adding a check to ensure thatvalue
within the slots is defined to prevent potentialundefined
issues invalue?.params
andvalue?.value
.Apply this diff to include default values:
return { componentName: 'template', props: { slot: { name: key, - params: value?.params || '' + params: (value && value.params) || '' } }, - children: value?.value + children: (value && value.value) || '' }
91-93
: Consistent return values intransformColumnToChildren
When
columns
is not an array, the function returnsundefined
. For consistency and to avoid unexpected behavior in calling functions, consider returning an empty array instead.Apply this diff:
- if (!Array.isArray(columns)) { - return - } + if (!Array.isArray(columns)) { + return [] + }
96-110
: Avoid mutation of input objectsIn
transformColumnToChildren
, you are destructuringitem
and modifyingrestItem
. To prevent side effects, it's better to create a deep copy ofitem
before modification.Apply this diff to clone
item
:const res = columns.map((item) => { + const clonedItem = { ...item } const { slots, ...restItem } = clonedItem let children = [] if (slots) { children = transformSlots(slots) } return { componentName: 'TinyGridColumn', props: restItem, children } })
116-123
: OptimizecolumnHasSlots
functionThe
columnHasSlots
function can return early upon finding the first column with slots. It's already efficient, but you can use thesome
method for cleaner code.Apply this diff:
-const columnHasSlots = (columns) => { - for (const columnItem of columns) { - if (columnItem.slots && typeof columnItem.slots === 'object' && Object.keys(columnItem.slots).length > 0) { - return true - } - } - return false -} +const columnHasSlots = (columns) => { + return columns.some( + (columnItem) => + columnItem.slots && + typeof columnItem.slots === 'object' && + Object.keys(columnItem.slots).length > 0 + ) +}packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json (2)
15-48
: Consider adding validation for numeric input.The employees field uses TinyNumeric but lacks input validation beyond the step constraint.
Consider adding min/max constraints and validation:
"componentName": "TinyNumeric", "props": { "allow-empty": true, "placeholder": "请输入", "controlsPosition": "right", "step": 1, + "min": 0, + "max": 999999, "modelValue": {
74-79
: Implement proper change handlers.The onChange handlers only log to console without actual implementation.
Would you like me to help implement proper change handlers for both numeric and text inputs? This should include:
- Data validation
- State updates
- Error handling
Also applies to: 35-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js
(1 hunks)packages/vue-generator/src/generator/vue/sfc/generateTemplate.js
(3 hunks)packages/vue-generator/test/testcases/sfc/case01/componentsMap.json
(1 hunks)packages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vue
(2 hunks)packages/vue-generator/test/testcases/sfc/slotModelValue/expected/slotModelValueTest.vue
(1 hunks)packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue-generator/test/testcases/sfc/slotModelValue/expected/slotModelValueTest.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (7)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (2)
13-13
: EnsuregetImportMap
is properly utilizedYou have imported
getImportMap
at line 13, but please verify that it's correctly utilized in the code to prevent any unused imports.
158-175
: Validate deletion ofprops.columns
after transformationAfter transforming
props.columns
to children components, you deleteprops.columns
. Ensure thatprops.columns
is not used elsewhere in the codebase after this deletion to prevent unintended side effects.Run the following script to search for usages of
props.columns
after this point:✅ Verification successful
props.columns
deletion is properly implementedThe deletion of
props.columns
is safe as it occurs:
- After all its usages are complete
- Only when slots transformation is performed
- Within a properly scoped context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if `props.columns` is used after deletion in `handleTinyGrid`. # Search for `props.columns` in files excluding `generateTemplate.js` rg "props\.columns" --glob '!generateTemplate.js'Length of output: 603
Script:
#!/bin/bash # Check usage of props.columns within generateTemplate.js rg "props\.columns" packages/vue-generator/src/generator/vue/sfc/generateTemplate.js -A 2 -B 2 # Check the function context where deletion occurs ast-grep --pattern 'function $FUNC($_) { $$$ delete props.columns $$$ }'Length of output: 714
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (1)
214-214
: Confirm the placement ofhandleTinyGrid
indefaultComponentHooks
Moving
handleTinyGrid
fromdefaultAttributeHook
todefaultComponentHooks
changes when this hook is executed. Ensure this change aligns with the intended hook execution order and that it doesn't affect other component processing logic.packages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vue (1)
57-58
: EnsureTinyIconDel
is correctly definedVerify that
TinyIconDel
is properly imported and initialized to avoid runtime errors when rendering the icon.packages/vue-generator/test/testcases/sfc/case01/componentsMap.json (1)
30-36
: LGTM! Verify component version compatibility.The addition of TinyGridColumn component follows the established patterns and aligns with the grid slot generation feature.
Run this script to verify the version compatibility with other @opentiny/vue components:
✅ Verification successful
Version compatibility verified ✓
The TinyGridColumn component uses version ^3.10.0, which is consistent with all other @opentiny/vue components in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check version compatibility across @opentiny/vue components # Expect: All components should use the same version rg -g '*.json' '"@opentiny/vue"' | grep '"version"' | sort | uniq -cLength of output: 70
Script:
#!/bin/bash # Find all componentsMap.json files and extract package + version info for @opentiny/vue fd componentsMap.json --exec sh -c 'echo "=== $1 ==="; jq -r ".[] | select(.package == \"@opentiny/vue\") | \"\(.componentName): \(.version)\"" {}' sh {}Length of output: 396
packages/vue-generator/test/testcases/sfc/slotModelValue/page.schema.json (2)
2-90
: LGTM! Well-structured state management for grid columns.The move to state-based column management improves maintainability and reusability.
119-122
: Implement error handling for undefined state.columns.The JSExpression accessing state.columns lacks error handling.
Consider adding a default empty array to prevent runtime errors:
"columns": { "type": "JSExpression", - "value": "state.columns" + "value": "state.columns || []" },
GridColumn as TinyGridColumn, | ||
Switch as TinySwitch | ||
} from '@opentiny/vue' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Import statements for icons may be missing
In your import statements, ensure that all icons used in the template are imported from '@opentiny/vue-icon'
. For example, IconDel
should be imported to define TinyIconDel
.
Apply this diff to include the missing imports:
import {
Button as TinyButton,
Form as TinyForm,
FormItem as TinyFormItem,
Grid as TinyGrid,
Input as TinyInput,
Select as TinySelect,
GridColumn as TinyGridColumn,
Switch as TinySwitch
-} from '@opentiny/vue'
+} from '@opentiny/vue'
+import { IconSearch, IconDel, iconHelpCircle, IconEdit } from '@opentiny/vue-icon'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
GridColumn as TinyGridColumn, | |
Switch as TinySwitch | |
} from '@opentiny/vue' | |
GridColumn as TinyGridColumn, | |
Switch as TinySwitch | |
} from '@opentiny/vue' | |
import { IconSearch, IconDel, iconHelpCircle, IconEdit } from '@opentiny/vue-icon' |
<tiny-grid :fetchData="{ api: getTableData }"> | ||
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column> | ||
<tiny-grid-column type="selection" :width="60"></tiny-grid-column> | ||
<tiny-grid-column field="employees" title="员工数"> | ||
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template | ||
></tiny-grid-column> | ||
<tiny-grid-column field="city" title="城市"></tiny-grid-column> | ||
<tiny-grid-column title="产品"> | ||
<template #default="{ row }"> | ||
<div> | ||
<tiny-switch modelValue=""></tiny-switch></div></template | ||
></tiny-grid-column> | ||
<tiny-grid-column title="操作"> | ||
<template #default=""> | ||
<tiny-button | ||
text="删除" | ||
:icon="TinyIconDel" | ||
@click="(...eventArgs) => emit(eventArgs, row)" | ||
></tiny-button></template></tiny-grid-column | ||
></tiny-grid> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix template syntax errors in tiny-grid-column
components
There are syntax errors in your template tags which may cause rendering issues:
- Missing closing angle brackets
>
in<template>
tags. - Incorrect closing of components.
Apply this diff to correct the syntax errors:
<tiny-grid :fetchData="{ api: getTableData }">
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column>
<tiny-grid-column type="selection" :width="60"></tiny-grid-column>
<tiny-grid-column field="employees" title="员工数">
- <template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template
+ <template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template>
</tiny-grid-column>
<tiny-grid-column field="city" title="城市"></tiny-grid-column>
<tiny-grid-column title="产品">
- <template #default="{ row }">
+ <template #default="{ row }">
<div>
<tiny-switch modelValue=""></tiny-switch>
- </div></template
+ </div>
+ </template>
</tiny-grid-column>
<tiny-grid-column title="操作">
- <template #default="">
+ <template #default>
<tiny-button
text="删除"
:icon="TinyIconDel"
@click="(...eventArgs) => emit(eventArgs, row)"
- ></tiny-button></template></tiny-grid-column
+ ></tiny-button>
+ </template>
+ </tiny-grid-column>
</tiny-grid>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<tiny-grid :fetchData="{ api: getTableData }"> | |
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column> | |
<tiny-grid-column type="selection" :width="60"></tiny-grid-column> | |
<tiny-grid-column field="employees" title="员工数"> | |
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template | |
></tiny-grid-column> | |
<tiny-grid-column field="city" title="城市"></tiny-grid-column> | |
<tiny-grid-column title="产品"> | |
<template #default="{ row }"> | |
<div> | |
<tiny-switch modelValue=""></tiny-switch></div></template | |
></tiny-grid-column> | |
<tiny-grid-column title="操作"> | |
<template #default=""> | |
<tiny-button | |
text="删除" | |
:icon="TinyIconDel" | |
@click="(...eventArgs) => emit(eventArgs, row)" | |
></tiny-button></template></tiny-grid-column | |
></tiny-grid> | |
<tiny-grid :fetchData="{ api: getTableData }"> | |
<tiny-grid-column type="index" :width="60" title=""></tiny-grid-column> | |
<tiny-grid-column type="selection" :width="60"></tiny-grid-column> | |
<tiny-grid-column field="employees" title="员工数"> | |
<template #default="{ row, rowIndex }"> <tiny-input></tiny-input></template> | |
</tiny-grid-column> | |
<tiny-grid-column field="city" title="城市"></tiny-grid-column> | |
<tiny-grid-column title="产品"> | |
<template #default="{ row }"> | |
<div> | |
<tiny-switch modelValue=""></tiny-switch> | |
</div> | |
</template> | |
</tiny-grid-column> | |
<tiny-grid-column title="操作"> | |
<template #default> | |
<tiny-button | |
text="删除" | |
:icon="TinyIconDel" | |
@click="(...eventArgs) => emit(eventArgs, row)" | |
></tiny-button> | |
</template> | |
</tiny-grid-column> | |
</tiny-grid> |
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
【问题描述】
如 issue #754 所描述,当表格组件使用插槽并为插槽里面的组件添加样式时,页面预览以及下载代码后,为插槽里面的组件添加的样式无法生效。
【问题分析】
vue setup 中的 jsx,无法自动添加 scoped id,但是添加的样式却是默认是 scoped id 的。导致样式无法应用。
【解决方案】
tinyvue grid 组件有插槽时,不直接在 setup 组件中生成 jsx,而是将表格列配置作为 tiny-grid-column 生成到 template 中。
即从生成 jsx :
改成生成代码为:
What is the current behavior?
Issue Number: #754
What is the new behavior?
Does this PR introduce a breaking change?
Other information