-
Notifications
You must be signed in to change notification settings - Fork 376
feat: 优化全局状态管理机制 #1305
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
base: develop
Are you sure you want to change the base?
feat: 优化全局状态管理机制 #1305
Conversation
1. 重构全局状态管理,将多个独立 store 合并为单一 globalState 2. 在 schema 中添加 globalState 引用,指向 stores 以支持全局状态访问 3. 修改变量配置器中的状态引用路径从 this.stores 改为 this.globalState 4. 更新页面生成器,在上下文中添加 globalState 支持 5. 优化 genGlobalState 插件,生成统一的全局状态管理文件 6. 在 lowcode.js 中集成 globalState 并暴露给组件使用 此更新使低代码平台具备更统一、更直观的全局状态管理能力,简化了状态访问路径。
WalkthroughThis pull request integrates a consolidated global state management approach across multiple modules. A new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Vue Component
participant S as useSchema
participant G as GlobalState Store
participant V as Vue Generator (page.js)
participant L as Lowcode Config
C->>S: Invoke useSchema(context with globalState)
Note right of S: globalState is now part of the context
V->>G: Retrieve global state via useGlobalState()
L->>G: Import global state for configuration
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
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: 0
🧹 Nitpick comments (3)
packages/vue-generator/src/plugins/genGlobalState.js (3)
44-60
: Consider more robust 'this' reference replacementThe current approach of replacing all occurrences of
this.
withthis.${id}.
may cause issues if 'this' appears in string literals or comments rather than actual code references.Consider using a more targeted approach for replacing 'this' references, possibly with regex that specifically targets property access patterns:
- if (gettersValue[key].value.includes('this')) { - gettersValue[key].value = gettersValue[key].value.replace(/this\./g, `this.${id}.`) + if (gettersValue[key].value.includes('this.')) { + gettersValue[key].value = gettersValue[key].value.replace(/\bthis\./g, `this.${id}.`)
66-72
: Improve code generation formatting for better readabilityThe current string concatenation approach for getters and actions might produce hard-to-read code in the generated files, especially for complex implementations.
Consider adding proper indentation:
- export const globalState = defineStore('globalState', { - state: () => (${JSON.stringify(state, null, 2)}), - getters: {${Object.entries(getters) - .map(([key, value]) => `${key}: ${value}`) - .join(',')}}, - actions: {${Object.entries(actions) - .map(([key, value]) => `${key}: ${value}`) - .join(',')}} - }) + export const globalState = defineStore('globalState', { + state: () => (${JSON.stringify(state, null, 2)}), + getters: { + ${Object.entries(getters) + .map(([key, value]) => `${key}: ${value}`) + .join(',\n ')} + }, + actions: { + ${Object.entries(actions) + .map(([key, value]) => `${key}: ${value}`) + .join(',\n ')} + } + })
75-80
: useGlobalState function may be redundantThe
useGlobalState
function doesn't seem to add significant value beyond whatglobalState()
already provides directly.Consider if this wrapper function is necessary, or if it will have additional functionality in the future. If it's meant to provide a consistent API, that's a valid reason to keep it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/canvas/render/src/page-block-function/schema.ts
(1 hunks)packages/configurator/src/variable-configurator/VariableConfigurator.vue
(1 hunks)packages/vue-generator/src/generator/page.js
(2 hunks)packages/vue-generator/src/plugins/genGlobalState.js
(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/canvas/render/src/page-block-function/schema.ts (1)
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js (1)
stores
(84-84)
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js (2)
packages/vue-generator/src/plugins/genGlobalState.js (1)
globalState
(9-9)packages/canvas/render/src/application-function/global-state.ts (1)
useGlobalState
(6-31)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (11)
packages/configurator/src/variable-configurator/VariableConfigurator.vue (1)
145-145
: LGTM: Updated reference to globalStateThe constant
STORE
has been updated from'this.stores.'
to'this.globalState.'
, which aligns with the PR objective of consolidating multiple stores into a single global state management approach.packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js (3)
20-20
: LGTM: Added import for useGlobalStateSuccessfully imported
useGlobalState
from the globalState module, enabling access to the consolidated global state management functionality.
85-85
: LGTM: Initialize globalStateAdded initialization of the
globalState
by callinguseGlobalState()
, which properly sets up the consolidated global state management.
87-87
: LGTM: Export globalState in returned objectThe
globalState
is now correctly included in the returned object alongside existing elements, making it accessible for component usage as intended.packages/canvas/render/src/page-block-function/schema.ts (1)
75-77
: LGTM: Added globalState to contextGood addition of the
globalState
parameter to the context object, with a clear comment explaining its purpose for global state management. This properly implements the PR objective of providing global state access within the schema.packages/vue-generator/src/generator/page.js (2)
410-410
: LGTM: Updated contextArr to include globalStateAdded
'globalState'
to thecontextArr
array, ensuring that the global state will be properly included in the wrapped context for component usage.
423-423
: LGTM: Added globalState to destructuring assignmentUpdated the destructuring assignment to include
globalState
from thevue.inject(I18nInjectionKey).lowcode()
method, making it available in the component's scope.packages/vue-generator/src/plugins/genGlobalState.js (4)
34-38
: Good setup for consolidated state managementThe new approach of using Pinia's
defineStore
with consolidated state, getters, and actions objects is a cleaner architecture than maintaining separate stores.
42-43
: Good defensive programming with default empty arraysProviding default empty arrays for getters and actions prevents potential undefined errors during processing.
84-95
: Good file organization and exportsThe approach to generate separate files for the store implementation and exports is clean and maintainable.
63-82
:✅ Verification successful
Potential issues with JSON.stringify for state serialization
Using
JSON.stringify
for the state object might cause problems with non-serializable values (functions, complex objects with circular references, etc.).Consider if any of your state values might contain functions or complex objects:
🏁 Script executed:
#!/bin/bash # Check for potential non-serializable values in state definitions rg -A 10 -B 10 "globalState" | grep -A 20 "state:" | grep -E "function|=>|\(\)|new [A-Z]"Length of output: 3505
Final Verification: Safe Use of JSON.stringify for Global State Serialization
After running the grep check for non-serializable constructs near the state definition in
packages/vue-generator/src/plugins/genGlobalState.js
, no occurrences of functions, arrow functions, ornew [A-Z]
patterns appeared. This indicates that the current usage ofJSON.stringify(state, null, 2)
is handling serializable data as expected.• The generated state appears to consist only of serializable values.
• For future changes, ensure that any new state properties remain serializable, or consider alternative approaches if non-serializable values are needed.
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
What is the current behavior?
调整了全局路由的管理方式,统一到一个module中,管理和使用更加简单
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor