fix: generate code accessor scenario #972
Conversation
修复 state accessor 出码的bug
|
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/test/testcases/sfc/accessor/expected/Accessor.vueOops! 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 attribute handling logic, specifically in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/vue-generator/test/testcases/sfc/accessor/accessor.test.js (2)
1-5: Use a named import for clarity.
While everything here is correct, consider making the imports more descriptive by grouping them or indicating their purpose, for example:-import { expect, test } from 'vitest' -import { genSFCWithDefaultPlugin } from '@/generator/vue/sfc' -import schema from './schema.json' -import { formatCode } from '@/utils/formatCode' +import { + expect, + test, +} from 'vitest' +import { + genSFCWithDefaultPlugin, +} from '@/generator/vue/sfc' +import schema from './schema.json' +import { + formatCode, +} from '@/utils/formatCode'This can improve maintainability and readability as the file grows.
6-11: Consider adding negative test cases.
The current test verifies the expected snapshot for the generated code. For more robust coverage, add scenarios with invalid or unexpected input to ensure the generator gracefully handles errors.packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue (3)
1-5: Localized text considerations.
"测试 state accessor 出码场景" appears as static text. If internationalization is intended throughout your application, consider wrapping this text with a translation function to maintain consistency.
19-30: Potential large-state complexity.
This reactive state object has numerous properties that transform on watchers. While this is acceptable for smaller-scale apps, consider factoring these properties or watchers into separate composables if the component grows in complexity to keep it modular.
33-67: Watchers reassigning properties.
All watchers use essentially the same logic: reassigning each state property to a concatenated string offirstNameandlastName. If the number of watchers grows, consider a single computed property or a composable approach to reduce duplication and potential drift across watchers.packages/vue-generator/test/testcases/sfc/accessor/schema.json (1)
1-14: Schema clarity.
The general structure is good. Including a brief comment explaining "Page" vs. "Accessor" naming helps maintain clarity for future contributors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js(2 hunks)packages/vue-generator/test/testcases/sfc/accessor/accessor.test.js(1 hunks)packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue(1 hunks)packages/vue-generator/test/testcases/sfc/accessor/schema.json(1 hunks)
🔇 Additional comments (3)
packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue (1)
7-17: Validate injection source.
Here, code is injecting from vue.inject(I18nInjectionKey). Ensure that I18nInjectionKey is provided at a higher level in the component tree to avoid runtime errors.
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (2)
382-384: Uniform return shape.
This addition ensures a uniform return object, preventing the code from crashing when non-object types are encountered. The implementation is valid.
418-429: Graceful handling of default values with accessors.
The transformation logic for defaultValue is robust, covering strings and nested objects. However, confirm that all accessor use-cases (e.g., arrays or computed properties) are tested.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue(1 hunks)packages/vue-generator/test/testcases/sfc/accessor/schema.json(1 hunks)
🔇 Additional comments (5)
packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue (3)
1-5: LGTM! Template structure matches schema definition.
19-30: LGTM! State initialization matches schema definition.
74-74: LGTM! Empty style section matches schema definition.
packages/vue-generator/test/testcases/sfc/accessor/schema.json (2)
1-14: LGTM! Basic component structure is well-defined.
The component structure, including name, filename, children, and empty sections for lifecycle and methods, is correctly defined.
Also applies to: 98-101
18-97:
Incorrect implementation of getters in state accessors.
There are several issues with the state accessor definitions:
- The "getter" functions are actually performing mutations instead of returning values
- All accessors convert values to strings, which may cause type-related issues
- The implementation pattern was previously flagged for the
objValproperty
Consider implementing proper getters that return computed values:
"nullValue": {
"defaultValue": null,
"accessor": {
"getter": {
"type": "JSFunction",
- "value": "function() { this.state.nullValue = `${this.state.firstName} ${this.state.lastName}` }"
+ "value": "function() { return null }"
}
}
},
"numberValue": {
"defaultValue": 0,
"accessor": {
"getter": {
"type": "JSFunction",
- "value": "function() { this.state.numberValue = `${this.state.firstName} ${this.state.lastName}` }"
+ "value": "function() { return this.state.firstName.length + this.state.lastName.length }"
}
}
}Let's verify if this pattern is used elsewhere in the codebase:
packages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vue
Show resolved
Hide resolved
| } | ||
| ], | ||
| "state": { | ||
| "firstName": "Opentiny", |
There was a problem hiding this comment.
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
What is the current behavior?
【问题描述】
状态变量包含 getter 或者 getter 的时候，初始化变量
defaultValue出码不正确。比如：
{ "state": { "test": { "defaultValue": "ccc", "accessor": { "getter": { "type": "JSFunction", "value": "this.state.test = this.props.xxx" } } } } }出码成：
【问题分析】
针对有 accessor 场景的
defaultValue没有进行分析处理。【解决方案】
针对 accessor 场景的
defaultValue，如果是复杂值，使用 transformObjType 方法进行递归处理。如果是字符串，需要进行转义。Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Accessor.vuewith various reactive state properties and a template.Tests
tagNamefunctionality in the Vue SFC generator.Bug Fixes