-
Notifications
You must be signed in to change notification settings - Fork 321
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
feat: merge lowcodeconfig with registry, migrate alias config to demo #551
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 (
|
aead370
to
37ebe14
Compare
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: 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
.reduce
accumulators can lead to performance issues due toO(n^2)
complexity. Consider using.push
or.splice
instead 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 ofgetMergeMeta
is correctly placed and follows standard practices.packages/controller/adapter.js (2)
14-14
: The import ofgetMergeMeta
is correctly placed and follows standard practices.
58-58
: The usage ofgetMergeMeta
to 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 andgetMergeMeta
are correctly placed and follow standard practices.
48-48
: The usage ofgetMergeMeta
to fetch the framework configuration is implemented correctly within the setup function.
49-49
: The usage ofgetMergeMeta
to fetch the platform configuration is implemented correctly within the setup function.packages/plugins/page/src/PageHistory.vue (1)
9-10
: LGTM! Ensure thatgetMergeMeta
correctly fetches the required configuration properties.Also applies to: 60-61
Verification successful
The search results indicate that
getMergeMeta
is widely used across various Vue files in the repository, includingpackages/plugins/page/src/PageHistory.vue
. The function is used to fetch configuration properties such asdslMode
andplatformId
, which aligns with the intended purpose of centralizing configuration management.
packages/plugins/page/src/PageHistory.vue
: The function is used to fetchdslMode
andplatformId
fromengine.config
.Given this context, it appears that
getMergeMeta
is 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 newconfig
object centralizes configuration settings effectively.Verification successful
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible from other parts of the system, we need to refine our search to instances whereconfig
is accessed as a property ofregistry
or in a similar context.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to expand our search to include different contexts where theconfig
object might be used.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to check if it is accessed within theinit
functions indesigner-demo/src/main.js
andpackages/design-core/src/init.js
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to confirm thatnewRegistry
inpackages/design-core/src/init.js
refers to theregistry
object fromdesigner-demo/registry.js
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to confirm thatregistry
indesigner-demo/src/main.js
refers to theregistry
object fromdesigner-demo/registry.js
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to confirm thatregistry
in./defineEntry.js
is exported fromdesigner-demo/registry.js
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to locate the correct file that exportsregistry
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to confirm whichdefineEntry.js
file is imported indesigner-demo/src/main.js
.
To ensure the new
config
object indesigner-demo/registry.js
is correctly integrated and accessible, we need to confirm the export ofregistry
indesigner-demo/src/defineEntry.js
.
The
registry
object is correctly exported fromdesigner-demo/src/defineEntry.js
and imported intodesigner-demo/src/main.js
. This confirms that theregistry
object, which includes the newconfig
object, 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 thatgetMergeMeta
correctly fetches theplatformId
.Also applies to: 30-30, 55-55
Verification successful
The
metasHashMap
is populated within thegenerateRegistry
function, which processes entries in a registry object. Theid
andvalue
pairs are added tometasHashMap
if theid
exists and the key is notmetaData
. This ensures thatgetMergeMeta
can retrieve theplatformId
frommetasHashMap
when it is populated correctly.The implementation appears to be correct and should function as intended.
packages/entry/src/common.js
: ThegenerateRegistry
function populatesmetasHashMap
with 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 replacedgetGlobalConfig
withgetMergeMeta
to align with new configuration management.
30-30
: Ensure thatplatformId
is always provided in the configuration to avoid runtime errors.packages/plugins/materials/src/block/http.js (1)
15-15
: Correctly implementedgetMergeMeta
for 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
: EnsuregetMergeMeta
is 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.js
anddark.js
exist 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
__dirname
and single quotes, and re-run the verification.
Let's modify the script to use
cd
andpwd
to 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
: EnsuregetMergeMeta
is 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
getMergeMeta
function 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
getMergeMeta
in the codebase and analyze its logic.
Let's use
rg
to search for thegetMergeMeta
function definition, which might handle the encoding issue better.
Let's search for the export statement of
getMergeMeta
to locate its definition.
Let's search for the definition of
getMergeMeta
within the@opentiny/tiny-engine-entry
module.
Let's review the implementation of
getMergeMeta
inpackages/entry/src/common.js
to verify its logic.
The
getMergeMeta
function retrieves data from ametasHashMap
using the providedid
. This means that the lifecycle options are dynamically retrieved based on theengine.config
key.
- The function
getMergeMeta
is defined as:export const getMergeMeta = (id) => { return metasHashMap[id] }This confirms that the lifecycle options are dynamically retrieved based on the
engine.config
key, 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
: EnsuregetMergeMeta
is 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 ofgetMergeMeta
aligns with the new configuration management strategy.
247-248
: Usage ofgetMergeMeta
ensures dynamic and consistent configuration access across the component.Also applies to: 277-277
packages/common/component/LifeCycles.vue (2)
82-82
: Import ofgetMergeMeta
aligns with the new configuration management strategy.
116-116
: Usage ofgetMergeMeta
ensures dynamic and consistent configuration access across the component.Also applies to: 118-118
packages/controller/src/hooks/useResource.js (2)
17-17
: ReplacegetGlobalConfig
withgetMergeMeta
to standardize configuration retrieval.This change aligns with the PR's objective to centralize and standardize configuration management.
253-253
: EnsuregetMergeMeta
is 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
: ReplacegetGlobalConfig
withgetMergeMeta
to standardize configuration retrieval.This change aligns with the PR's objective to centralize and standardize configuration management.
152-152
: EnsuregetMergeMeta
is 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.config
is referenced in multiple files across the codebase, including the file under review (packages/controller/src/hooks/useBlock.js
). This indicates thatengine.config
is a recognized configuration entity within the project.
packages/controller/src/hooks/useBlock.js
: Lines 335 and 357 referenceengine.config
fordslMode
.Given this context, the use of
getMergeMeta('engine.config')?.dslMode
appears 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
…opentiny#551) * feat: merge lowcodeconfig with registry, migrate alias config to demo * fix: delete useless config file * fix(theme): load theme pkg ASAP
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
getGlobalConfig
withgetMergeMeta
across multiple components and hooks for improved configuration management.Chores