feat: merge lowcodeconfig with registry, migrate alias config to demo #551
Conversation
WalkthroughThe changes primarily involve replacing the usage of Changes
Sequence Diagram(s) (Beta)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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
chilingling
force-pushed
the
feat/designCoreConfigExtract
branch
from
aead370
to
37ebe14
Compare
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (3)
packages/plugins/materials/src/block/http.js (1)
Line range hint
58-61: Optimize the method chain by using.flatMap()instead of.map().flat().- const blocks = blockGroups - .map((group) => group.blocks.map((block) => ({ ...block, groupId: group.id, groupName: group.name }))) - .flat() - .filter(({ framework, id }) => { - if (dslMode === framework && !blockIdSet.has(id)) { - blockIdSet.add(id) - return true - } - return false - }) + const blocks = blockGroups.flatMap((group) => + group.blocks + .filter(({ framework, id }) => dslMode === framework && !blockIdSet.has(id)) + .map((block) => { + blockIdSet.add(id); + return { ...block, groupId: group.id, groupName: group.name }; + }) + );packages/canvas/src/components/render/runner.js (1)
Line range hint
125-125: Consider using optional chaining to avoid potential runtime errors when accessing properties on undefined objects.- const { styles: thirdStyles = [], scripts: thirdScripts = [] } = window.thirdPartyDeps || {} + const { styles: thirdStyles = [], scripts: thirdScripts = [] } = window.thirdPartyDeps ?? {}packages/controller/src/hooks/useResource.js (1)
Line range hint
432-433: Optimize the use of spread syntax in accumulators.Using spread syntax in
.reduceaccumulators can lead to performance issues due toO(n^2)complexity. Consider using.pushor.spliceinstead to improve performance.- return { ...componentsMap, [component.componentName]: component } + componentsMap[component.componentName] = component; + return componentsMap;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (27)
- designer-demo/registry.js (1 hunks)
- designer-demo/vite.config.js (1 hunks)
- packages/canvas/src/components/render/runner.js (1 hunks)
- packages/common/component/LifeCycles.vue (2 hunks)
- packages/controller/adapter.js (2 hunks)
- packages/controller/src/hooks/useBlock.js (3 hunks)
- packages/controller/src/hooks/useResource.js (2 hunks)
- packages/controller/src/hooks/useSaveLocal.js (3 hunks)
- packages/controller/src/index.js (2 hunks)
- packages/design-core/config/lowcode.config.js (1 hunks)
- packages/design-core/registry.js (1 hunks)
- packages/design-core/src/App.vue (2 hunks)
- packages/design-core/src/init.js (2 hunks)
- packages/design-core/src/preview/src/App.vue (1 hunks)
- packages/design-core/src/theme/dark.js (1 hunks)
- packages/design-core/src/theme/light.js (1 hunks)
- packages/design-core/vite.config.js (4 hunks)
- packages/plugins/block/src/BlockSetting.vue (3 hunks)
- packages/plugins/bridge/src/BridgeSetting.vue (2 hunks)
- packages/plugins/datasource/src/DataSourceTemplate.vue (1 hunks)
- packages/plugins/i18n/src/Main.vue (2 hunks)
- packages/plugins/materials/src/block/http.js (4 hunks)
- packages/plugins/page/src/PageHistory.vue (2 hunks)
- packages/settings/props/src/components/groups/LifeCycle.vue (3 hunks)
- packages/toolbars/generate-vue/src/Main.vue (2 hunks)
- packages/toolbars/logo/src/Main.vue (2 hunks)
- packages/toolbars/preview/src/Main.vue (2 hunks)
Files not reviewed due to errors (1)
- packages/toolbars/logo/src/Main.vue (no review received)
Files skipped from review due to trivial changes (4)
- packages/controller/src/index.js
- packages/design-core/src/theme/dark.js
- packages/design-core/src/theme/light.js
- packages/design-core/vite.config.js
Additional context used
Biome
packages/plugins/materials/src/block/http.js
[error] 58-61: The call chain .map().flat() can be replaced with a single .flatMap() call. (lint/complexity/useFlatMap)
Safe fix: Replace the chain with .flatMap().
packages/canvas/src/components/render/runner.js
[error] 125-125: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
packages/controller/src/hooks/useResource.js
[error] 432-433: Avoid the use of spread (
...) syntax on accumulators. (lint/performance/noAccumulatingSpread)Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.
Additional comments not posted (30)
packages/design-core/src/preview/src/App.vue (1)
7-7: The import ofgetMergeMetais correctly placed and follows standard practices.packages/controller/adapter.js (2)
14-14: The import ofgetMergeMetais correctly placed and follows standard practices.
58-58: The usage ofgetMergeMetato determine the editor's theme is implemented correctly and safely uses optional chaining.packages/toolbars/preview/src/Main.vue (3)
20-21: The import statements for hooks andgetMergeMetaare correctly placed and follow standard practices.
48-48: The usage ofgetMergeMetato fetch the framework configuration is implemented correctly within the setup function.
49-49: The usage ofgetMergeMetato fetch the platform configuration is implemented correctly within the setup function.packages/plugins/page/src/PageHistory.vue (1)
9-10: LGTM! Ensure thatgetMergeMetacorrectly fetches the required configuration properties.Also applies to: 60-61
Verification successful
The search results indicate that
getMergeMetais widely used across various Vue files in the repository, includingpackages/plugins/page/src/PageHistory.vue. The function is used to fetch configuration properties such asdslModeandplatformId, which aligns with the intended purpose of centralizing configuration management.
packages/plugins/page/src/PageHistory.vue: The function is used to fetchdslModeandplatformIdfromengine.config.Given this context, it appears that
getMergeMetais correctly implemented and used consistently across the codebase to fetch configuration properties.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `getMergeMeta` is correctly implemented and returns expected configuration properties. # Test: Search for the function usage and ensure it returns the expected properties. rg --type vue $'getMergeMeta'Length of output: 56
Script:
#!/bin/bash # Description: Verify that `getMergeMeta` is correctly implemented and returns expected configuration properties. # Test: Search for the function usage and ensure it returns the expected properties. rg 'getMergeMeta' --glob '**/*.vue'Length of output: 3701
designer-demo/registry.js (1)
14-18: LGTM! The newconfigobject centralizes configuration settings effectively.Verification successful
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible from other parts of the system, we need to refine our search to instances whereconfigis accessed as a property ofregistryor in a similar context.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to expand our search to include different contexts where theconfigobject might be used.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to check if it is accessed within theinitfunctions indesigner-demo/src/main.jsandpackages/design-core/src/init.js.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to confirm thatnewRegistryinpackages/design-core/src/init.jsrefers to theregistryobject fromdesigner-demo/registry.js.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to confirm thatregistryindesigner-demo/src/main.jsrefers to theregistryobject fromdesigner-demo/registry.js.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to confirm thatregistryin./defineEntry.jsis exported fromdesigner-demo/registry.js.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to locate the correct file that exportsregistry.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to confirm whichdefineEntry.jsfile is imported indesigner-demo/src/main.js.
To ensure the new
configobject indesigner-demo/registry.jsis correctly integrated and accessible, we need to confirm the export ofregistryindesigner-demo/src/defineEntry.js.
The
registryobject is correctly exported fromdesigner-demo/src/defineEntry.jsand imported intodesigner-demo/src/main.js. This confirms that theregistryobject, which includes the newconfigobject, is integrated and accessible from other parts of the system.
designer-demo/src/defineEntry.js:export { registry }designer-demo/src/main.js:import { registry } from './defineEntry.js'Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new `config` object is correctly integrated and accessible from other parts of the system. # Test: Search for the usage of `config` properties across the project. rg --type js $'config'Length of output: 61417
Script:
#!/bin/bash # Description: Verify that the new `config` object in `designer-demo/registry.js` is correctly integrated and accessible from other parts of the system. # Test: Search for the usage of `registry.config` across the project. rg --type js 'registry.config'Length of output: 30
Script:
#!/bin/bash # Description: Verify that the new `config` object in `designer-demo/registry.js` is correctly integrated and accessible from other parts of the system. # Test: Search for the usage of `config` within the `registry` object or similar contexts. rg --type js 'registry.*config'Length of output: 245
Script:
#!/bin/bash # Description: Verify that the new `config` object in `designer-demo/registry.js` is correctly integrated and accessible from other parts of the system. # Test: Search for the usage of `config` within the `init` functions in `designer-demo/src/main.js` and `packages/design-core/src/init.js`. rg --type js 'config' designer-demo/src/main.js packages/design-core/src/init.jsLength of output: 529
Script:
#!/bin/bash # Description: Verify that `newRegistry` in `packages/design-core/src/init.js` refers to the `registry` object from `designer-demo/registry.js`. # Test: Search for the initialization of `newRegistry` in `packages/design-core/src/init.js`. rg --type js 'newRegistry' packages/design-core/src/init.jsLength of output: 208
Script:
#!/bin/bash # Description: Verify that `registry` in `designer-demo/src/main.js` refers to the `registry` object from `designer-demo/registry.js`. # Test: Search for the initialization of `registry` in `designer-demo/src/main.js`. rg --type js 'registry' designer-demo/src/main.jsLength of output: 127
Script:
#!/bin/bash # Description: Verify that `registry` in `./defineEntry.js` refers to the `registry` object from `designer-demo/registry.js`. # Test: Search for the export of `registry` in `./defineEntry.js`. rg --type js 'registry' designer-demo/defineEntry.jsLength of output: 121
Script:
#!/bin/bash # Description: Locate the correct file that exports `registry`. # Test: Search for files named `defineEntry.js` or similar in the project. fd defineEntry.jsLength of output: 100
Script:
#!/bin/bash # Description: Confirm which `defineEntry.js` file is imported in `designer-demo/src/main.js`. # Test: Search for the import statement in `designer-demo/src/main.js`. rg --type js 'defineEntry' designer-demo/src/main.jsLength of output: 194
Script:
#!/bin/bash # Description: Confirm the export of `registry` in `designer-demo/src/defineEntry.js`. # Test: Search for the export statement in `designer-demo/src/defineEntry.js`. rg --type js 'export' designer-demo/src/defineEntry.jsLength of output: 74
packages/design-core/src/init.js (1)
49-50: LGTM! Ensure that sensitive configuration data is not exposed globally.packages/controller/src/hooks/useSaveLocal.js (1)
14-14: LGTM! Ensure thatgetMergeMetacorrectly fetches theplatformId.Also applies to: 30-30, 55-55
Verification successful
The
metasHashMapis populated within thegenerateRegistryfunction, which processes entries in a registry object. Theidandvaluepairs are added tometasHashMapif theidexists and the key is notmetaData. This ensures thatgetMergeMetacan retrieve theplatformIdfrommetasHashMapwhen it is populated correctly.The implementation appears to be correct and should function as intended.
packages/entry/src/common.js: ThegenerateRegistryfunction populatesmetasHashMapwith the necessary configuration data, includingplatformId.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `getMergeMeta` is correctly implemented and returns the expected `platformId`. # Test: Search for the function usage and ensure it returns the expected property. rg --type js $'getMergeMeta'Length of output: 5356
Script:
#!/bin/bash # Description: Extract the implementation of `getMergeMeta` from `packages/entry/src/common.js`. # Extract the function implementation ast-grep --lang javascript --pattern $'export const getMergeMeta = $_ => $_' packages/entry/src/common.jsLength of output: 271
Script:
#!/bin/bash # Description: Search for the definition and initialization of `metasHashMap` in the codebase. # Search for `metasHashMap` definition and initialization rg --type js 'metasHashMap'Length of output: 204
Script:
#!/bin/bash # Description: Extract the code that populates `metasHashMap` in `packages/entry/src/common.js`. # Extract the relevant code ast-grep --lang javascript --pattern $'metasHashMap[$_] = $_' packages/entry/src/common.jsLength of output: 157
Script:
#!/bin/bash # Description: Extract the surrounding code where `metasHashMap` is populated in `packages/entry/src/common.js`. # Extract the surrounding code sed -n '140,160p' packages/entry/src/common.jsLength of output: 531
packages/plugins/datasource/src/DataSourceTemplate.vue (2)
21-21: Correctly replacedgetGlobalConfigwithgetMergeMetato align with new configuration management.
30-30: Ensure thatplatformIdis always provided in the configuration to avoid runtime errors.packages/plugins/materials/src/block/http.js (1)
15-15: Correctly implementedgetMergeMetafor dynamic configuration access in HTTP request functions.Also applies to: 41-41, 55-55, 81-81
packages/canvas/src/components/render/runner.js (1)
138-138: Correctly implemented dynamic handling of canvas dependencies. Ensure that all dependencies are correctly formatted and accessible.packages/design-core/src/App.vue (2)
43-43: EnsuregetMergeMetais imported from the correct module.
50-53: Verify the dynamic theme import path.Verification successful
The dynamic theme import path is correct, as the required theme files
light.jsanddark.jsexist in the./packages/design-core/src/theme/directory.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dynamic theme import path is correct. # Test: Search for the theme files. Expect: Files should exist. fd --type file --exec ls {} \; | grep -E 'theme/(light|dark)\.js$'Length of output: 154
designer-demo/vite.config.js (1)
8-59: Verify the correctness of the alias paths.Verification successful
Let's correct the script to properly handle
__dirnameand single quotes, and re-run the verification.
Let's modify the script to use
cdandpwdto resolve the full paths, and re-run the verification.
The verification script executed successfully and did not produce any errors, indicating that all the alias paths in the Vite configuration file are correct and exist in the filesystem.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the alias paths in the Vite configuration. # Test: Check if the paths resolve correctly. Expect: No errors. for path in $(grep 'path.resolve' designer-demo/vite.config.js | cut -d'(' -f2 | cut -d')' -f1); do if [ ! -d "$path" ] && [ ! -f "$path" ]; then echo "Path $path does not exist." fi doneLength of output: 29687
Script:
#!/bin/bash # Description: Verify the correctness of the alias paths in the Vite configuration. # Test: Check if the paths resolve correctly. Expect: No errors. grep 'path.resolve' designer-demo/vite.config.js | while read -r line; do path=$(echo "$line" | sed -E "s/.*path\.resolve\(__dirname, '(.*)'\).*/\1/") full_path=$(realpath "designer-demo/$path") if [ ! -d "$full_path" ] && [ ! -f "$full_path" ]; then echo "Path $full_path does not exist." fi doneLength of output: 24906
Script:
#!/bin/bash # Description: Verify the correctness of the alias paths in the Vite configuration. # Test: Check if the paths resolve correctly. Expect: No errors. grep 'path.resolve' designer-demo/vite.config.js | while read -r line; do path=$(echo "$line" | sed -E "s/.*path\.resolve\(__dirname, '(.*)'\).*/\1/") full_path=$(cd "designer-demo/$(dirname "$path")" && pwd)/$(basename "$path") if [ ! -d "$full_path" ] && [ ! -f "$full_path" ]; then echo "Path $full_path does not exist." fi doneLength of output: 30071
packages/settings/props/src/components/groups/LifeCycle.vue (2)
78-78: EnsuregetMergeMetais imported from the correct module.
106-106: Verify the dynamic retrieval of lifecycle options.Also applies to: 122-122
Verification successful
To verify the dynamic retrieval of lifecycle options, we need to inspect the implementation of the
getMergeMetafunction and ensure it correctly handles the'engine.config'parameter. This will help us understand if the lifecycle options are being retrieved as expected.Let's search for the implementation of
getMergeMetain the codebase and analyze its logic.
Let's use
rgto search for thegetMergeMetafunction definition, which might handle the encoding issue better.
Let's search for the export statement of
getMergeMetato locate its definition.
Let's search for the definition of
getMergeMetawithin the@opentiny/tiny-engine-entrymodule.
Let's review the implementation of
getMergeMetainpackages/entry/src/common.jsto verify its logic.
The
getMergeMetafunction retrieves data from ametasHashMapusing the providedid. This means that the lifecycle options are dynamically retrieved based on theengine.configkey.
- The function
getMergeMetais defined as:export const getMergeMeta = (id) => { return metasHashMap[id] }This confirms that the lifecycle options are dynamically retrieved based on the
engine.configkey, as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dynamic retrieval of lifecycle options is correct. # Test: Check if the lifecycle options are retrieved correctly. Expect: Correct data structure. echo "Retrieving lifecycle options..." echo $(grep 'getMergeMeta' packages/settings/props/src/components/groups/LifeCycle.vue | cut -d'(' -f2 | cut -d')' -f1)Length of output: 404
Script:
#!/bin/bash # Search for the implementation of the getMergeMeta function in the codebase ast-grep --lang javascript --pattern 'function getMergeMeta($_) { $$$ }'Length of output: 165
Script:
#!/bin/bash # Search for the implementation of the getMergeMeta function in the codebase using ripgrep rg 'function getMergeMeta' --glob '!node_modules/*'Length of output: 53
Script:
#!/bin/bash # Search for the export statement of the getMergeMeta function in the codebase using ripgrep rg 'export function getMergeMeta' --glob '!node_modules/*'Length of output: 60
Script:
#!/bin/bash # Search for the definition of getMergeMeta within the @opentiny/tiny-engine-entry module rg 'getMergeMeta' --glob '!node_modules/*'Length of output: 5371
Script:
#!/bin/bash # Display the implementation of getMergeMeta in packages/entry/src/common.js rg 'export const getMergeMeta' -A 10 packages/entry/src/common.jsLength of output: 133
packages/toolbars/generate-vue/src/Main.vue (2)
27-27: EnsuregetMergeMetais imported from the correct module.
59-60: Verify the dynamic retrieval of platform and DSL mode configurations.packages/plugins/block/src/BlockSetting.vue (2)
82-82: Import ofgetMergeMetaaligns with the new configuration management strategy.
247-248: Usage ofgetMergeMetaensures dynamic and consistent configuration access across the component.Also applies to: 277-277
packages/common/component/LifeCycles.vue (2)
82-82: Import ofgetMergeMetaaligns with the new configuration management strategy.
116-116: Usage ofgetMergeMetaensures dynamic and consistent configuration access across the component.Also applies to: 118-118
packages/controller/src/hooks/useResource.js (2)
17-17: ReplacegetGlobalConfigwithgetMergeMetato standardize configuration retrieval.This change aligns with the PR's objective to centralize and standardize configuration management.
253-253: EnsuregetMergeMetais used correctly for fetching material configurations.This usage is consistent with the new approach to configuration management, ensuring that material configurations are dynamically fetched based on the current engine configuration.
packages/plugins/i18n/src/Main.vue (2)
127-128: ReplacegetGlobalConfigwithgetMergeMetato standardize configuration retrieval.This change aligns with the PR's objective to centralize and standardize configuration management.
152-152: EnsuregetMergeMetais used correctly for theme determination.This usage is consistent with the new approach to configuration management, ensuring that theme settings are dynamically fetched based on the current engine configuration.
packages/controller/src/hooks/useBlock.js (1)
335-335: Verify the existence and correct configuration ofengine.config.Also applies to: 357-357
Verification successful
The search results confirm that
engine.configis referenced in multiple files across the codebase, including the file under review (packages/controller/src/hooks/useBlock.js). This indicates thatengine.configis a recognized configuration entity within the project.
packages/controller/src/hooks/useBlock.js: Lines 335 and 357 referenceengine.configfordslMode.Given this context, the use of
getMergeMeta('engine.config')?.dslModeappears to be consistent with the rest of the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and correct configuration of `engine.config`. # Test: Search for the definition of `engine.config`. Expect: At least one definition. rg --type json --type js --type vue 'engine.config'Length of output: 79
Script:
#!/bin/bash # Description: Verify the existence and correct configuration of `engine.config`. # Test: Search for the definition of `engine.config`. Expect: At least one definition. rg --type json --type js --type ts 'engine.config'Length of output: 3884
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- designer-demo/registry.js (1 hunks)
- packages/design-core/src/init.js (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- designer-demo/registry.js
- packages/design-core/src/init.js
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
New Features
Refactor
getGlobalConfigwithgetMergeMetaacross multiple components and hooks for improved configuration management.Chores