fix: slot should extra params #919
Conversation
WalkthroughThe changes involve the addition of a new function, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (2)
139-155: Consider adding documentation and unit testsThis new function handles an important aspect of slot parameter processing but lacks documentation and tests.
Add JSDoc documentation explaining:
- Function purpose
- Expected input format
- Side effects (props mutation)
- Example usage
Add unit tests covering:
- Valid parameter processing
- Invalid input handling
- Edge cases (empty params, duplicate names)
Would you like me to help generate the documentation and unit test cases?
139-155: Consider a more functional approachThe current implementation mutates the props object directly. Consider a more functional approach that returns a new props object.
-export const handleSlotParams = (schemaData) => { +export const handleSlotParams = (schemaData) => { + const { componentName, props } = schemaData.schema + + if (componentName !== 'Slot' || !props?.params || !Array.isArray(props.params)) { + return + } + + const newProps = { ...props } + delete newProps.params + + props.params.forEach((paramItem) => { + const { name, value } = paramItem || {} + if (name && value) { + newProps[name] = value + } + }) + + schemaData.schema.props = newProps +}This approach:
- Makes the mutation more explicit
- Makes it easier to track changes
- Follows functional programming principles
- Makes the code more testable
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js(2 hunks)packages/vue-generator/src/generator/vue/sfc/generateTemplate.js(1 hunks)
🔇 Additional comments (2)
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (2)
9-10: LGTM: Import of new slot params handler
The import of handleSlotParams is correctly added alongside other template generation hooks.
217-217: Verify hook execution order
handleSlotParams is placed at the beginning of the defaultAttributeHook array, meaning it will execute before other attribute hooks. Please confirm this ordering is intentional, as it could affect how subsequent hooks process the modified props.
✅ Verification successful
Let me gather more information about the hook execution order and slot parameter handling.
Let me check how the other hooks interact with props to understand if the ordering is critical.
Based on the gathered information, I can now provide the final response:
The placement of handleSlotParams at the beginning is correct and intentional
The hook ordering is appropriate because:
handleSlotParamstransforms slot parameters into direct props and removes theparamsproperty- Subsequent hooks like
handleTinyGrid,handleJsxModelValueUpdate, andhandleConditionAttrHookwork with the transformed props structure - No other hooks depend on or expect the original
paramsstructure to be present
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other hooks depend on or modify slot params
# Look for any references to slot params in other attribute hooks
rg -l "props\.params" --type js
Length of output: 145
Script:
#!/bin/bash
# Check the implementation of handleSlotParams and other attribute hooks
ast-grep --pattern 'const handleSlotParams = $_'
# Look for the defaultAttributeHook array definition and its contents
ast-grep --pattern 'const defaultAttributeHook = $_'
# Check for any direct references to handleSlotParams
rg "handleSlotParams" -A 3 --type js
Length of output: 4140
Script:
#!/bin/bash
# Check implementations of subsequent hooks that might depend on props
ast-grep --pattern 'const handleTinyGrid = $_'
ast-grep --pattern 'const handleJsxModelValueUpdate = $_'
ast-grep --pattern 'const handleConditionAttrHook = $_'
# Look for any modifications to props in these hooks
rg "props\." -A 2 -B 2 packages/vue-generator/src/generator/vue/sfc/generateTemplate.js
Length of output: 9257
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsPage.vue (5)
4-5: Consider adding null checks for slot propsThe template directly accesses
testData.namewithout checking iftestDataexists. Consider adding a defensive check to handle cases where the slot prop might be undefined.- <span>{{ testData.name }}</span></template + <span>{{ testData?.name }}</span></template
12-13: Optimize Vue imports
definePropsanddefineEmitsare already available in the script setup scope without explicit imports. Additionally, they're being imported twice (viavuenamespace and directly).-import * as vue from 'vue' -import { defineProps, defineEmits } from 'vue' +import { inject, reactive } from 'vue'
17-19: Remove unnecessary empty definitionsSince both
propsandemitare empty, these definitions can be omitted unless they're required by the lowcode framework.
22-25: Consider consolidating wrap callsMultiple
wrapcalls with different objects might be consolidated into a single call. Also, the empty reactive state appears unused.-wrap({ stores }) - -const state = vue.reactive({}) -wrap({ state }) +const state = vue.reactive({}) +wrap({ stores, state })
27-27: Remove empty style blockConsider removing the empty scoped style block to reduce file size and improve readability.
packages/vue-generator/test/testcases/sfc/slotParams/page.schema.json (1)
24-34: Enhance test coverage for slot parameter expressions.The current test only verifies simple property access (
testData.name). Consider adding test cases for:
- Complex expressions using slot parameters
- Multiple slot parameters in a single expression
- Method calls on slot parameters
- Destructured slot parameters
packages/vue-generator/test/testcases/sfc/slotParams/block.schema.json (1)
3-8: Consider data type and i18n improvements for personData.The state structure could be improved in several ways:
- Age should be a number instead of string:
"age": 20- Personal information is hardcoded in Chinese, consider using i18n keys
"state": { "personData": { - "name": "李华", - "age": "20", + "name": "{{ t('person.name') }}", + "age": 20, "address": "china" } }packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsBlock.vue (2)
3-3: Use i18n for hardcoded text.Replace hardcoded Chinese text with i18n translation.
- <span>请开启插槽</span> + <span>{{ t('slots.enable') }}</span>
11-13: Optimize imports.The code imports
definePropsanddefineEmitsseparately even though they're available from the main Vue import.import * as vue from 'vue' -import { defineProps, defineEmits } from 'vue' import { I18nInjectionKey } from 'vue-i18n'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js(1 hunks)packages/vue-generator/test/testcases/sfc/slotParams/block.schema.json(1 hunks)packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsBlock.vue(1 hunks)packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsPage.vue(1 hunks)packages/vue-generator/test/testcases/sfc/slotParams/page.schema.json(1 hunks)packages/vue-generator/test/testcases/sfc/slotParams/slotParams.test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue-generator/src/generator/vue/sfc/generateTemplate.js
🔇 Additional comments (5)
packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsPage.vue (1)
1-27: Verify test coverage for slot parameters
Since this is a test case for a bug fix related to slot parameters, ensure that edge cases are covered in the test suite.
✅ Verification successful
Test coverage for slot parameters is adequate
The test suite demonstrates comprehensive coverage for slot parameters:
- Tests both block and page components
- Verifies slot parameter passing with realistic data (
testDatawith nested properties) - Includes snapshot testing for both components
- Covers the complete flow from schema to rendered component
The implementation correctly handles:
- Named slots with parameters (
#test="{ testData }") - Reactive state binding to slot props (
:testData="state.personData") - Component composition with slot content
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related test cases to ensure comprehensive coverage
echo "Searching for related slot parameter test cases..."
rg -l "slotParams" --type vue --type js packages/vue-generator/test/
# Check for potential error handling tests
echo "Checking for error handling test cases..."
rg -l "test.*error.*slot|test.*invalid.*slot" packages/vue-generator/test/
Length of output: 377
Script:
#!/bin/bash
# Search for related slot parameter test cases with correct file extensions
echo "Searching for related slot parameter test cases..."
fd . packages/vue-generator/test/ -e vue -e js -x rg -l "slotParams" {}
# Check test files containing slot-related tests
echo "Checking slot-related test files..."
fd . packages/vue-generator/test/ -e vue -e js -x rg -l "slot" {}
# Look for test configuration or test suite files
echo "Checking test suite files..."
fd . packages/vue-generator/test/ -e js -x rg -l "describe.*slot|test.*slot" {}
Length of output: 1310
Script:
#!/bin/bash
# Let's examine the content of the slot parameter test file to verify coverage
echo "Content of slot parameter test file:"
cat packages/vue-generator/test/testcases/sfc/slotParams/slotParams.test.js
# Check the test components to understand test scenarios
echo -e "\nContent of test components:"
cat packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsBlock.vue
Length of output: 1666
packages/vue-generator/test/testcases/sfc/slotParams/page.schema.json (3)
1-8: Consider expanding test coverage with non-empty component properties.
While this serves as a minimal test case, consider adding variations with non-empty state, methods, props, or lifecycle hooks to ensure the slot parameter handling works correctly with other component features.
9-14: Verify slot behavior with component props.
The BlockSlotParams component has empty props. Consider adding test cases that verify slot parameter behavior when the component also has props to ensure there are no naming conflicts or scope issues.
15-23: Add test cases for multiple slot parameters.
The current test only covers a single slot parameter. Consider adding test cases for:
- Multiple parameters in a single slot
- Multiple slots with different parameters
- Nested slots with parameters
This would ensure the handleSlotParams function handles all scenarios correctly.
packages/vue-generator/test/testcases/sfc/slotParams/block.schema.json (1)
31-41: Verify slot parameter binding syntax.
The slot parameter binding uses this.state.personData, but in Vue 3's composition API context, this might not be available. Ensure this expression works correctly in the generated component.
packages/vue-generator/test/testcases/sfc/slotParams/page.schema.json
Outdated
Show resolved
Hide resolved
packages/vue-generator/test/testcases/sfc/slotParams/expected/slotParamsBlock.vue
Show resolved
Hide resolved
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
区块出码增加插槽传参解析
当前区块如果有插槽和传参，schema 片段可能是这样的：
{ "children": [ { "componentName": "Slot", "props": { // 插槽名称 "name": "test" // 往插槽参数里面的传参 "params": [ { "name": "testData", "value": { "type": "JSExpression", "value": "this.state.testData" } } ] } } ] }如果不处理，会默认出码成：
需要增加处理插槽传参，出码成为：
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
TinyGridcomponent to handle properties more effectively.Documentation