fix: 优化全局状态处理和资源加载逻辑 #1207
fix: 优化全局状态处理和资源加载逻辑 #1207
Conversation
- 重构 global-state.ts 中的状态处理，支持数组和对象类型的状态计算 - 修改 VariableConfigurator.vue 中的变量加载方式 - 更新 useResource 中的应用状态获取逻辑，增加兼容性
WalkthroughThis pull request refines state management across multiple components. In the global state module, the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant GS as useGlobalState Function
Caller->>GS: Invoke useGlobalState(state, getters)
GS->>GS: Check if state is an array
alt State is array
GS->>GS: If no getters, copy state directly
alt Getters exist
GS->>GS: Iterate over getter keys with try–catch
GS->>GS: Build arrayWithGetters
end
GS->>GS: Assign result to stores[id]
else
GS->>GS: Check if state is a valid object
alt Valid Object
GS->>GS: If no getters, merge state directly
alt Getters exist
GS->>GS: Compute getters with try–catch and merge with state
end
else
GS->>GS: Assign state directly to stores[id]
end
end
GS-->>Caller: Return updated store
sequenceDiagram
participant F as fetchAppState Function
participant Schema as appSchemaState Object
F->>F: Retrieve appData.meta
alt Using camelCase for isDemo
F->>Schema: Set isDemo from appData.meta.isDemo
else
F->>Schema: Set isDemo from appData.meta.is_demo
end
alt Using camelCase for globalState
F->>Schema: Set globalState from appData.meta.globalState
else
F->>Schema: Set globalState from appData.meta.global_state
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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/canvas/render/src/application-function/global-state.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/canvas".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/canvas/.eslintrc.cjs » @vue/eslint-config-typescript". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/plugins/materials/src/composable/useResource.js (1)
146-147: Enhanced fallback logic for metadata property access.The implementation now supports both camelCase and snake_case naming conventions for metadata properties, improving compatibility and making the code more robust.
Consider creating a utility function for this pattern if it's used frequently throughout the codebase:
+ const getMetaProp = (obj, camelCaseProp, snakeCaseProp) => obj?.[camelCaseProp] || obj?.[snakeCaseProp]; - appSchemaState.isDemo = appData.meta?.isDemo || appData.meta?.is_demo - appSchemaState.globalState = appData?.meta.globalState || appData?.meta.global_state + appSchemaState.isDemo = getMetaProp(appData.meta, 'isDemo', 'is_demo') + appSchemaState.globalState = getMetaProp(appData?.meta, 'globalState', 'global_state')packages/canvas/render/src/application-function/global-state.ts (1)
16-52: Improved global state handling with type differentiation and error protection.This refactoring significantly enhances the robustness of global state management by:
- Properly handling different state types (arrays, objects, primitives)
- Adding try-catch blocks to protect against errors in getter functions
- Preserving the original structure when adding computed getters
This is a substantial improvement that prevents potential runtime errors and ensures consistent state behavior across different data structures.
Consider extracting the repeated getter computation logic into a helper function to reduce code duplication:
+ const computeGettersWithErrorHandling = (getters, state) => { + const computedGetters = {}; + Object.keys(getters).forEach((key) => { + try { + computedGetters[key] = new Func('return ' + getters[key])().call(computedGetters, state); + } catch (error) { + computedGetters[key] = undefined; + } + }); + return computedGetters; + }; if (Array.isArray(state)) { if (!hasGetters) { stores[id] = [...state]; } else { - const computedGetters = {}; - Object.keys(getters).forEach((key) => { - try { - computedGetters[key] = new Func('return ' + getters[key])().call(computedGetters, state); - } catch (error) { - computedGetters[key] = undefined; - } - }); + const computedGetters = computeGettersWithErrorHandling(getters, state); const arrayWithGetters = [...state]; Object.assign(arrayWithGetters, computedGetters); stores[id] = arrayWithGetters; } } else if (typeof state !== 'object' || state === null) { stores[id] = state; } else { if (!hasGetters) { stores[id] = { ...state }; } else { - const computedGetters = {}; - Object.keys(getters).forEach((key) => { - try { - computedGetters[key] = new Func('return ' + getters[key])().call(computedGetters, state); - } catch (error) { - computedGetters[key] = undefined; - } - }); + const computedGetters = computeGettersWithErrorHandling(getters, state); stores[id] = Object.assign({}, state, computedGetters); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/canvas/render/src/application-function/global-state.ts(1 hunks)packages/configurator/src/variable-configurator/VariableConfigurator.vue(1 hunks)packages/plugins/materials/src/composable/useResource.js(1 hunks)
🔇 Additional comments (1)
packages/configurator/src/variable-configurator/VariableConfigurator.vue (1)
508-519:❓ Verification inconclusive
Simplified store variable handling.
The code now directly assigns store IDs to
state.variables[id]instead of creating nested property bindings. This simplification aligns with the comment that store value processing has been migrated toglobal-state.ts.Since the commented code indicates the store value processing was moved to
global-state.ts, make sure the UI correctly displays store values after this change:
🏁 Script executed:
#!/bin/bash # Check if there are UI components that might depend on the old property binding format rg -A 5 "this\.stores\.\w+\.\w+" --type vueLength of output: 73
Simplified store variable handling – Verify UI Compatibility
The code now directly assigns store IDs to
state.variables[id], in line with the migration of store value processing toglobal-state.ts. However, note that our attempt to verify UI components using the search command ran into an issue due to file type recognition (i.e. "vue" files were not automatically recognized). Please verify that no Vue components are still expecting a nested property binding (such as patterns likethis.stores.<store>.<property>). For example, you might run:#!/bin/bash # Re-run the search with an explicit pattern for Vue files rg -g '*.vue' -A 5 "this\.stores\.\w+\.\w+"If any such references are found, ensure they are updated accordingly.
| stores[id] = { ...state, ...computedGetters } | ||
| const hasGetters = Object.keys(getters).length > 0 | ||
|
|
||
| if (Array.isArray(state)) { |
There was a problem hiding this comment.
可以给一下相关的场景用例数据嘛？我们结合来评估一下，麻烦啦~
There was a problem hiding this comment.
原来的代码，新建一个应用状态，如： testList: ["One", "Two", "Three", "Four"]，变量列表中会展示四个变量： testList.0、testList.1、testList.2、testList.3
如果是字符串，如 demo: "Hello"，变量列表会得到 demo.0/demo.1/……
| Object.keys(getters).forEach(loadProp) | ||
| stores.forEach(({ id, state: _storeState = {}, _getters = {} }) => { | ||
| state.variables[id] = id | ||
| // fix: store 列表错误渲染成子属性列表 |
There was a problem hiding this comment.
原代码中，直接赋值为 store()，出码后，变量的值变成了 store 实例，而不是所需的变量值
| Object.keys(storeState).forEach(loadProp) | ||
| Object.keys(getters).forEach(loadProp) | ||
| stores.forEach(({ id, state: _storeState = {}, _getters = {} }) => { | ||
| state.variables[id] = id |
There was a problem hiding this comment.
【bug 场景】：
当全局应用状态设置为数组时，绑定变量弹窗处分割的状态不正确。
【origin 状态分割的场景】
应用的状态可能有很多的字段，为了方便用户快速绑定对应的字段，这里做了优化。
比如一个应用状态可能是 object，有很多的字段，可能还有 getter：
所以这里仅特殊处理一下数组的场景会好一点？
| @@ -4,7 +4,7 @@ const useStores = () => { | |||
| const stores = {} | |||
|
|
|||
| Object.values({ ...useDefinedStores }).forEach((store) => { | |||
| stores[store.$id] = store() | |||
| stores[store.$id] = store().$state | |||
There was a problem hiding this comment.
请问这里改成 $state 是有什么 bug 场景不满足吗？
这里应该不仅仅只赋值 store 的 state。
因为我们要允许用户从 this.stores.storeName 读取 state 或者是 action 或者是 gettter state。
比如：
this.stores.test.name读取 statethis.stores.test.fullName读取 getterStatethis.stores.test.setName拿到 stores 的 action
| Object.assign(arrayWithGetters, computedGetters) | ||
| stores[id] = arrayWithGetters | ||
| } | ||
| } else if (typeof state !== 'object' || state === null) { |
There was a problem hiding this comment.
我们将全局状态的 state 限制为 plain object 会不会好一点？
因为我们这里的实现是基于 pinia，但是 pinia 要求我们 state 返回一个初始状态：
https://pinia.vuejs.org/zh/core-concepts/state.html
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?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit