Skip to content
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

refactor: extract material-related code to useMaterial from useResource #591

Merged
merged 11 commits into from
Jul 1, 2024

Conversation

wenmine
Copy link
Collaborator

@wenmine wenmine commented Jun 22, 2024

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Refactor
    • Updated internal method from useResource to useMaterial for enhanced retrieval of third-party dependencies.

@wenmine wenmine changed the base branch from develop to refactor/develop June 22, 2024 09:45
Copy link
Contributor

coderabbitai bot commented Jun 22, 2024

Walkthrough

The recent update primarily focuses on replacing the useResource function with the useMaterial function from @opentiny/tiny-engine-meta-register across several files. This impacts how materials, including third-party dependencies for scripts and styles, are handled and retrieved in various parts of the project.

Changes

File/Path Change Summary
packages/common/js/preview.js Replaced useResource with useMaterial for managing third-party dependencies retrieval.
packages/settings/props/src/composable/use...js Replaced useResource with useMaterial in the getProps function to fetch appropriate materials.

Sequence Diagram

sequenceDiagram
    participant Component as Preview Component
    participant ResourceModule as @opentiny/tiny-engine-meta-register
    Component->>ResourceModule: useMaterial()
    alt Material Found
        ResourceModule-->>Component: Return Material
        Note right of Component: Material retrieved successfully
    else Material Not Found
        ResourceModule-->>Component: Return Error
        Note right of Component: Handle error or alternative scenario
    end
Loading

Poem

🐇 In the world of code, a change we see,
Functions shift like leaves from tree,
useResource bids farewell,
useMaterial rings the bell,
Dependencies now align,
Code flows smoothly, oh so fine! ✨


Tip

Early access features: enabled

We are currently testing the following features in early access:

  • OpenAI gpt-4o model for code reviews and chat: OpenAI claims that this model is better at understanding and generating code than the previous models. We seek your feedback over the next few weeks before making it generally available.

Note:

  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.
  • Please join our Discord Community to provide feedback and report issues.
  • OSS projects are currently opted into early access features by default.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (7)
packages/settings/props/src/composable/useProperties.js (1)

Line range hint 219-222: Suggestion: Use optional chaining for safer property access.

Consider using optional chaining to simplify property access and enhance code safety. This will help avoid potential runtime errors when properties might be undefined.

- const { props } = useProperties().getSchema()
+ const props = useProperties().getSchema()?.props || {}
packages/plugins/materials/src/composable/useMaterial.js (1)

Line range hint 323-323: Optimize accumulator usage in reduce function.

Using spread syntax in a reducer function can lead to performance issues due to repeated copying of objects. Consider using Object.assign or other methods to mutate the accumulator directly.

-    componentState.componentsMap = appData.componentsMap?.reduce((componentsMap, component) => {
-      if (component.dependencies) {
-        getBlockDeps(component.dependencies)
-      }
-      return { ...componentsMap, [component.componentName]: component }
-    }, {})
+    componentState.componentsMap = appData.componentsMap?.reduce((componentsMap, component) => {
+      if (component.dependencies) getBlockDeps(component.dependencies);
+      componentsMap[component.componentName] = component;
+      return componentsMap;
+    }, {});
Tools
Biome

[error] 49-50: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 132-155: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 237-238: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

packages/plugins/materials/src/composable/useResource.js (4)

Line range hint 61-62: Refactor assignment within expression.

Similar to the previous file, assignments within expressions can lead to confusion and errors. It's better to separate the logic for clarity and maintainability.

- const schema = {
-   componentName: component,
-   props: {},
-   ...getSnippet(component)
- }
+ const snippet = getSnippet(component);
+ const schema = {
+   componentName: component,
+   props: {},
+   ...snippet
+ }

Line range hint 144-167: Remove redundant else clause.

The else clause is unnecessary here because the if condition ends with a return statement, making the else redundant and less readable.

-  if (notFetchResouce) {
-    return block
-  } else {
-    if (!blockResource.get(label)) {
+  if (!notFetchResouce && !blockResource.get(label)) {

Line range hint 251-252: Remove unnecessary else clause.

The else clause is redundant after a return statement in the preceding if block. This can simplify the control flow and improve readability.

-  if (response.status === 'fulfilled' && response.value.materials) {
-    addMaterials(response.value.materials)
-  }
+  if (response.status === 'fulfilled' && response.value.materials) addMaterials(response.value.materials);

Line range hint 437-439: Refactor assignment within expression.

Using assignments within expressions can lead to confusion and errors. It's better to separate the logic for clarity and maintainability.

- const schema = {
-   componentName: component,
-   props: {},
-   ...getSnippet(component)
- }
+ const snippet = getSnippet(component);
+ const schema = {
+   componentName: component,
+   props: {},
+   ...snippet
+ }
packages/canvas/container/src/container.js (1)

Line range hint 239-247: Remove Unnecessary Else Clause

The static analysis tool has flagged an unnecessary else clause. Removing it can simplify the control flow and improve code readability.

- else {
-   type = POSITION.BOTTOM
- }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 54b8353 and 1742734.

Files selected for processing (28)
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue (2 hunks)
  • packages/canvas/container/src/CanvasContainer.vue (2 hunks)
  • packages/canvas/container/src/components/CanvasAction.vue (2 hunks)
  • packages/canvas/container/src/components/shortCutPopover.vue (2 hunks)
  • packages/canvas/container/src/container.js (2 hunks)
  • packages/common/component/ConfigItem.vue (2 hunks)
  • packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue (2 hunks)
  • packages/configurator/src/slot-configurator/SlotConfigurator.vue (2 hunks)
  • packages/controller/js/preview.js (2 hunks)
  • packages/controller/src/index.js (1 hunks)
  • packages/design-core/index.js (1 hunks)
  • packages/entry/src/hooks.js (3 hunks)
  • packages/plugins/materials/index.js (1 hunks)
  • packages/plugins/materials/src/block/BlockGroupPanel.vue (2 hunks)
  • packages/plugins/materials/src/block/BlockList.vue (2 hunks)
  • packages/plugins/materials/src/block/Main.vue (3 hunks)
  • packages/plugins/materials/src/component/Main.vue (2 hunks)
  • packages/plugins/materials/src/composable/Untitled-1.ts (1 hunks)
  • packages/plugins/materials/src/composable/index.js (2 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
  • packages/plugins/materials/src/composable/useResource.js (6 hunks)
  • packages/plugins/page/src/Main.vue (3 hunks)
  • packages/plugins/tree/src/Main.vue (2 hunks)
  • packages/settings/events/src/components/BindEvents.vue (2 hunks)
  • packages/settings/props/src/components/inputs/BindFunction.vue (2 hunks)
  • packages/settings/props/src/components/inputs/DraggableOptions.vue (3 hunks)
  • packages/settings/props/src/composable/useProperties.js (2 hunks)
  • packages/toolbars/refresh/src/Main.vue (2 hunks)
Files skipped from review due to trivial changes (1)
  • packages/controller/src/index.js
Additional context used
Biome
packages/controller/js/preview.js

[error] 31-31: 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 of O(n^2).
Consider methods such as .splice or .push instead.

packages/plugins/materials/src/composable/Untitled-1.ts

[error] 60-60: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)

The last required parameter is here:

A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.

packages/settings/props/src/composable/useProperties.js

[error] 219-222: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

packages/plugins/materials/src/composable/useMaterial.js

[error] 49-50: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 132-155: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 237-238: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 323-323: 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 of O(n^2).
Consider methods such as .splice or .push instead.

packages/plugins/materials/src/composable/useResource.js

[error] 61-62: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 144-167: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 251-252: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 437-439: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

packages/canvas/container/src/container.js

[error] 239-247: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

Additional comments not posted (37)
packages/plugins/materials/src/composable/index.js (1)

14-21: New MaterialService definition approved.

The addition of MaterialService aligns with the project's move from useResource to useMaterial. The definition is consistent with existing service patterns in the codebase.

packages/plugins/materials/index.js (1)

15-24: Export and integration of MaterialService approved.

The addition of MaterialService to the exports and its inclusion in the metas array are correctly implemented. This change supports the modular architecture of the application.

packages/controller/js/preview.js (1)

Line range hint 15-28: Use of useMaterial approved.

The integration of useMaterial is correctly implemented, aligning with the project's shift from useResource to useMaterial.

packages/toolbars/refresh/src/Main.vue (1)

Line range hint 19-39: Integration of useMaterial and refresh logic improvements approved.

The use of useMaterial for clearing block resources and refreshing canvas elements is correctly implemented, enhancing the modularity and maintainability of the code.

packages/settings/props/src/components/inputs/BindFunction.vue (2)

15-15: Update import statements to include useMaterial.

The import statement now correctly includes useMaterial, aligning with the project's shift from useResource to useMaterial. Ensure that all functionalities previously handled by useResource are now properly managed by useMaterial.


37-37: Correct usage of useMaterial for material handling.

The useMaterial hook is used to fetch material-related data, which is a part of the new functionality introduced in the application. Verify that the getMaterial method provides all the necessary data previously handled by useResource.

Verification successful

Correct usage of useMaterial for material handling.

The getMaterial method in both useMaterial.js and useResource.js has identical implementations. This confirms that the getMaterial method in useMaterial provides all the necessary data previously handled by useResource. Therefore, the transition to useMaterial should be seamless without any loss of functionality.

  • packages/plugins/materials/src/composable/useResource.js
  • packages/plugins/materials/src/composable/useMaterial.js
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `getMaterial` method provides all necessary data.
# Test: Search for the `getMaterial` usage in the project to ensure it covers all data aspects.
rg --type vue $'getMaterial'

Length of output: 55



Script:

#!/bin/bash
# Corrected script to search for `getMaterial` usage across the project.
rg 'getMaterial'

Length of output: 3125



Script:

#!/bin/bash
# Extract the implementation of `getMaterial` in `useMaterial.js` and `useResource.js` for comparison.
ast-grep --lang javascript --pattern 'const getMaterial = $_' --json

Length of output: 1961

packages/design-core/index.js (1)

18-18: Properly added MaterialService to exports.

The inclusion of MaterialService alongside Materials and ResourceService in the exports ensures that all related functionalities are accessible from this module. This change supports the broader refactor of material handling in the application.

packages/entry/src/hooks.js (1)

21-22: Addition of new hooks for material handling.

The new hooks useMaterial and useCustom have been added correctly. This aligns with the application's shift towards more granular control over materials and custom functionalities. Ensure that these hooks are integrated throughout the application where needed.

Also applies to: 45-45

Verification successful

Verified integration of useMaterial and useCustom hooks across the application.

The useMaterial and useCustom hooks have been correctly integrated and are being utilized in various parts of the application, as evidenced by the search results.

  • Files with useMaterial and useCustom usage:
    • packages/configurator/src/slot-configurator/SlotConfigurator.vue
    • packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue
    • packages/toolbars/refresh/src/Main.vue
    • packages/plugins/tree/src/Main.vue
    • packages/plugins/materials/src/composable/useResource.js
    • packages/plugins/materials/src/composable/index.js
    • packages/plugins/materials/src/component/Main.vue
    • packages/plugins/materials/src/block/Main.vue
    • packages/plugins/materials/src/block/BlockList.vue
    • packages/plugins/materials/src/block/BlockGroupPanel.vue
    • packages/settings/props/src/components/inputs/BindFunction.vue
    • packages/settings/props/src/composable/useProperties.js
    • packages/settings/events/src/components/BindEvents.vue
    • packages/controller/src/index.js
    • packages/controller/js/preview.js
    • packages/common/component/ConfigItem.vue
    • packages/canvas/container/src/CanvasContainer.vue
    • packages/canvas/container/src/container.js
    • packages/canvas/container/src/components/shortCutPopover.vue
    • packages/canvas/container/src/components/CanvasAction.vue
    • packages/canvas/DesignCanvas/src/DesignCanvas.vue
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify integration of `useMaterial` and `useCustom` hooks across the application.
# Test: Search for usages of these hooks to ensure they are being utilized where necessary.
rg --type js $'useMaterial|useCustom'

Length of output: 5727

packages/canvas/container/src/components/shortCutPopover.vue (2)

33-33: Updated import statement to use useMaterial.

The import statement now includes useMaterial, which aligns with the project's shift from using useResource to useMaterial. Ensure that all methods previously accessed through useResource are available and correctly implemented in useMaterial.


42-42: Correct implementation of useMaterial.

The getMaterial method is retrieved from useMaterial() and used to fetch material configurations based on components. This usage is consistent with the intended functionality of useMaterial. Verify that getMaterial provides all necessary data previously handled by useResource.

packages/plugins/page/src/Main.vue (1)

38-38: Updated import statement to include necessary modules.

The import now correctly includes useCanvas, useApp, usePage, and useHelp from @opentiny/tiny-engine-controller. This change is essential for the file's functionality and aligns with the architectural modifications in the project.

packages/plugins/materials/src/block/Main.vue (2)

15-15: Updated import statement to use useMaterial.

The import statement correctly includes useMaterial from @opentiny/tiny-engine-controller, which is part of the project's shift towards a more material-centric architecture.


29-29: Correct usage of useMaterial in state management and data fetching.

The materialState from useMaterial is used effectively to manage block data. This is a significant change, ensuring that material data is integrated into the block management logic. Verify that all data interactions are correctly handled and that there are no regressions in functionality.

Also applies to: 70-77

packages/settings/props/src/components/inputs/DraggableOptions.vue (1)

28-28: Updated import statement to use useProperties.

The import statement correctly includes useProperties from @opentiny/tiny-engine-controller, ensuring that property management functionalities are accessible within the component.

packages/configurator/src/slot-configurator/SlotConfigurator.vue (2)

28-28: Approved new imports.

The inclusion of useMaterial aligns with the overall PR objectives and the shift from useResource to useMaterial.


111-111: Good integration of useMaterial.

The use of useMaterial to fetch material configurations within toggleSlot method is well-integrated and supports the new functionality introduced by the PR.

packages/plugins/materials/src/component/Main.vue (2)

37-37: Approved updated imports for material handling.

The inclusion of useMaterial and useCanvas aligns with the shift towards a more material-centric architecture.


52-52: Effective use of useMaterial for dynamic component handling.

The usage of useMaterial for deriving generateNode and managing materialState is crucial for the dynamic functionality of the component panel.

packages/settings/props/src/composable/useProperties.js (1)

15-15: Approved the integration of useMaterial in property management.

The inclusion of useMaterial alongside useCanvas and useTranslate is well-suited for the new material-centric functionality.

packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue (1)

51-51: Approved the integration of useMaterial for attribute configuration.

The inclusion of useMaterial aligns with the replacement of useResource, enhancing the material management capabilities of the attribute configurator.

packages/plugins/materials/src/block/BlockList.vue (2)

20-20: Updated imports for new material handling functionality.

The code now imports useMaterial from the tiny-engine-controller, which is part of the effort to shift from useResource to useMaterial. This change aligns with the project's goal to better manage material-related functionalities.


52-52: Refactor to use useMaterial for material management functions.

The usage of generateNode and registerBlock from useMaterial is appropriate for the component's functionality related to handling blocks. This change is consistent with the overall project's transition from resource to material management.

packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)

23-23: Updated imports to include useMaterial.

The addition of useMaterial is part of the broader refactoring to use the new material management module, which is a positive change, ensuring that material-related functionalities are handled more robustly.


185-187: Use of useMaterial methods in component methods.

The methods getMaterial and registerBlock from useMaterial are now being used to handle material-specific operations within the canvas. This is a good use of the new module, ensuring that material operations are centralized and consistent.

packages/plugins/materials/src/block/BlockGroupPanel.vue (2)

27-27: Refactoring to include useMaterial in imports.

The inclusion of useMaterial alongside useResource suggests a transitional phase where both resource and material functionalities might still be needed. This is a critical step in migrating fully to the useMaterial module.


116-116: Integration of useMaterial for updating canvas dependencies.

The use of useMaterial().updateCanvasDependencies to handle canvas dependencies after adding blocks is a significant improvement, ensuring that material dependencies are managed efficiently.

packages/canvas/container/src/CanvasContainer.vue (2)

33-33: Updated imports to include useMaterial.

The addition of useMaterial to the imports is in line with the project's shift towards a more material-centric architecture. This change supports the enhanced management of material-related functionalities within the canvas.


113-113: Use of useMaterial to manage third-party dependencies in the canvas.

Integrating useMaterial().materialState.thirdPartyDeps to manage third-party dependencies directly within the canvas setup ensures that material dependencies are handled efficiently and are up-to-date.

packages/plugins/materials/src/composable/useMaterial.js (1)

237-240: Remove unnecessary else clause.

The else clause is redundant after a return statement in the preceding if block. This can simplify the control flow and improve readability.
[REFACTOR_SUGGESTion]

-  if (response.status === 'fulfilled' && response.value.materials) {
-    addMaterials(response.value.materials)
-  }
+  if (response.status === 'fulfilled' && response.value.materials) addMaterials(response.value.materials);
Tools
Biome

[error] 237-238: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

packages/settings/events/src/components/BindEvents.vue (1)

Line range hint 90-113: Update imports and API usage to use useMaterial.

The code correctly updates the imports and API usage to incorporate useMaterial, aligning with the overall project's shift from useResource to useMaterial.

packages/plugins/tree/src/Main.vue (1)

Line range hint 80-102: Ensure proper use of useMaterial.

The file correctly replaces useResource with useMaterial, which is part of the overall refactor in the application. This change is consistent with the new approach to managing materials.

packages/common/component/ConfigItem.vue (2)

109-109: Update to use useMaterial.

The import statement correctly includes useMaterial from @opentiny/tiny-engine-controller. This change aligns with the PR's goal to transition from useResource to useMaterial.


257-257: Ensure proper handling of material data.

The usage of useMaterial().getMaterial(currentComponent) is crucial for fetching material-specific data. Ensure that currentComponent is always defined and valid in this context to prevent runtime errors.

packages/canvas/container/src/components/CanvasAction.vue (2)

131-131: Update to use useMaterial.

The import statement correctly includes useMaterial from @opentiny/tiny-engine-controller. This change aligns with the PR's goal to transition from useResource to useMaterial.


239-239: Ensure proper handling of material data.

The usage of useMaterial().getMaterial(props.selectState.componentName) is crucial for fetching material-specific data. Ensure that props.selectState.componentName is always defined and valid in this context to prevent runtime errors.

packages/canvas/container/src/container.js (2)

22-22: Update Import Statement to Include useMaterial

The import statement has been updated to include useMaterial from @opentiny/tiny-engine-controller. This change is consistent with the project's shift from useResource to useMaterial, as outlined in the PR summary.


920-920: Updated Dependency Management to Use useMaterial

The dependency update function call has been changed to use useMaterial().materialState.thirdPartyDeps instead of useResource().resState.thirdPartyDeps. This aligns with the broader refactoring goals of the PR to transition from resource-based to material-based management.

packages/controller/js/preview.js Outdated Show resolved Hide resolved
packages/plugins/materials/src/composable/Untitled-1.ts Outdated Show resolved Hide resolved
packages/plugins/materials/src/composable/useMaterial.js Outdated Show resolved Hide resolved
packages/plugins/materials/src/composable/useMaterial.js Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
packages/canvas/container/src/container.js (1)

Line range hint 239-247: Remove redundant else clause.

The else clause is unnecessary here because the if condition ends with a return statement, making the else redundant and less readable.

-  if (doc) {
-    doc.dispatchEvent(new CustomEvent(name, data))
-  } else {
-    return
-  }
+  if (doc) {
+    doc.dispatchEvent(new CustomEvent(name, data))
+  }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1742734 and 8a166b3.

Files selected for processing (7)
  • packages/canvas/container/src/container.js (2 hunks)
  • packages/common/component/MetaContainer.vue (2 hunks)
  • packages/common/component/MetaTableColumns.vue (1 hunks)
  • packages/plugins/block/src/js/blockSetting.jsx (2 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
  • packages/plugins/materials/src/composable/useResource.js (5 hunks)
  • packages/settings/props/src/components/modal/ModalContent.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/plugins/materials/src/composable/useResource.js
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 49-50: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 142-164: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)


[error] 261-261: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)


[error] 359-359: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 350-351: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

packages/plugins/block/src/js/blockSetting.jsx

[error] 489-489: This default parameter should follow the last required parameter or should be a required parameter.

The last required parameter is here:

A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.

(lint/style/useDefaultParameterLast)

packages/canvas/container/src/container.js

[error] 239-247: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Additional comments not posted (13)
packages/common/component/MetaTableColumns.vue (2)

7-7: Ensure unused imports are removed.

While the change looks good, ensure that all unused imports are removed to keep the code clean.


11-11: LGTM!

The usage of useMaterial to get the configure map is correctly implemented.

packages/common/component/MetaContainer.vue (2)

25-25: Ensure unused imports are removed.

While the change looks good, ensure that all unused imports are removed to keep the code clean.


36-36: LGTM!

The usage of useMaterial to get the configure map is correctly implemented.

packages/settings/props/src/components/modal/ModalContent.vue (1)

47-47: Update to use useMaterial instead of useResource.

The import should be updated to use useMaterial to maintain consistency with the refactor.

- import { useResource } from '@opentiny/tiny-engine-controller'
+ import { useMaterial } from '@opentiny/tiny-engine-controller'

Likely invalid or redundant comment.

packages/plugins/materials/src/composable/useMaterial.js (6)

271-288: LGTM!


295-303: LGTM!


247-249: LGTM!


207-229: LGTM!


251-254: Avoid using await within a loop.

Using await within a loop can lead to performance issues due to sequential execution. Consider using Promise.all for concurrent execution.

-  const materials = await Promise.allSettled(bundleUrls.map((url) => http.get(url)))
+  const materials = await Promise.allSettled(bundleUrls.map((url) => http.get(url)))

Likely invalid or redundant comment.


261-263: Remove redundant else clause.

The else clause is unnecessary here because the if condition ends with a return statement, making the else redundant and less readable.

-    if (response.status === 'fulfilled' && response.value.materials) {
-      addMaterials(response.value.materials)
-    }
+    if (response.status === 'fulfilled' && response.value.materials) {
+      addMaterials(response.value.materials)
+    }

Likely invalid or redundant comment.

Tools
Biome

[error] 261-261: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

packages/plugins/block/src/js/blockSetting.jsx (1)

53-53: LGTM!

packages/canvas/container/src/container.js (1)

919-920: LGTM!

packages/plugins/materials/src/composable/useMaterial.js Outdated Show resolved Hide resolved
packages/plugins/materials/src/composable/useMaterial.js Outdated Show resolved Hide resolved
packages/canvas/container/src/container.js Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range and nitpick comments (1)
packages/plugins/block/src/js/blockSetting.jsx (1)

Line range hint 489-489: Fix parameter order in function definition.

This default parameter should follow the last required parameter or should be a required parameter.

- const createOrUpdateCategory = async ({ categoryId, ...params }, isEdit) => {
+ const createOrUpdateCategory = async (isEdit, { categoryId, ...params }) => {
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8a166b3 and 13fa603.

Files selected for processing (13)
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue (2 hunks)
  • packages/common/component/ConfigItem.vue (2 hunks)
  • packages/configurator/src/container-configurator/ContainerConfigurator.vue (2 hunks)
  • packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue (1 hunks)
  • packages/entry/src/hooks.js (3 hunks)
  • packages/plugins/block/src/js/blockSetting.jsx (2 hunks)
  • packages/plugins/materials/index.js (2 hunks)
  • packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (2 hunks)
  • packages/plugins/materials/src/meta/block/src/BlockList.vue (2 hunks)
  • packages/plugins/materials/src/meta/block/src/Main.vue (3 hunks)
  • packages/plugins/materials/src/meta/component/src/Main.vue (2 hunks)
  • packages/settings/events/src/components/BindEvents.vue (2 hunks)
  • packages/settings/props/src/composable/useProperties.js (2 hunks)
Files skipped from review as they are similar to previous changes (4)
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue
  • packages/common/component/ConfigItem.vue
  • packages/plugins/materials/index.js
  • packages/settings/events/src/components/BindEvents.vue
Additional context used
Biome
packages/entry/src/hooks.js

[error] 75-75: The comma operator is disallowed.

Its use is often confusing and obscures side effects.

(lint/style/noCommaOperator)

packages/settings/props/src/composable/useProperties.js

[error] 219-222: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/plugins/block/src/js/blockSetting.jsx

[error] 489-489: This default parameter should follow the last required parameter or should be a required parameter.

The last required parameter is here:

A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.

(lint/style/useDefaultParameterLast)

Additional comments not posted (17)
packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue (1)

7-11: LGTM!

The refactor to use useMaterial instead of useResource looks good. The changes integrate well with the existing code.

packages/configurator/src/container-configurator/ContainerConfigurator.vue (1)

Line range hint 25-36: LGTM!

The refactor to use useMaterial instead of useResource looks good. The changes integrate well with the existing code.

packages/plugins/materials/src/meta/block/src/Main.vue (4)

19-19: Update import statement.

The useResource function has been replaced with useMaterial from @opentiny/tiny-engine-controller.


43-43: Update function usage.

The materialState is now being retrieved from useMaterial.


84-84: Update function usage.

The blocks property is now accessed from materialState.


91-91: Update function usage.

The blocks property is now accessed from materialState.

packages/plugins/materials/src/meta/component/src/Main.vue (3)

37-37: Update import statement.

The useResource function has been replaced with useMaterial from @opentiny/tiny-engine-controller.


52-52: Update function usage.

The generateNode and materialState are now being retrieved from useMaterial.


55-55: Update function usage.

The components property is now accessed from materialState.

packages/settings/props/src/composable/useProperties.js (2)

15-15: Update import statement.

The useResource function has been replaced with useMaterial from @opentiny/tiny-engine-entry.


163-163: Update function usage.

The getMaterial function is now being retrieved from useMaterial.

packages/plugins/materials/src/meta/block/src/BlockList.vue (2)

20-20: Update import statement to use useMaterial.

The import statement has been updated to replace useResource with useMaterial.


52-52: Update function calls to use useMaterial.

The function calls have been updated to use useMaterial instead of useResource.

packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (2)

27-27: Update import statement to use useMaterial.

The import statement has been updated to replace useResource with useMaterial.


116-116: Update function call to use useMaterial.

The function call has been updated to use useMaterial instead of useResource.

packages/plugins/block/src/js/blockSetting.jsx (2)

27-27: Update import statement to use useMaterial.

The import statement has been updated to replace useResource with useMaterial.


53-53: Update function call to use useMaterial.

The function call has been updated to use useMaterial instead of useResource.

packages/entry/src/hooks.js Outdated Show resolved Hide resolved
@wenmine wenmine changed the title Refactor/use material useResource解耦第一版:将物料相关资源从useResource中剥离 Jun 29, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 13fa603 and 8ac2a2b.

Files selected for processing (2)
  • packages/controller/js/preview.js (2 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/controller/js/preview.js
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 49-50: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 257-257: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)


[error] 355-355: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 346-347: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

Additional comments not posted (13)
packages/plugins/materials/src/composable/useMaterial.js (13)

39-47: LGTM!


68-73: LGTM!


75-84: LGTM!


86-99: LGTM!


148-152: LGTM!


154-155: LGTM!


160-190: LGTM!


197-201: LGTM!


203-225: LGTM!


228-240: LGTM!


243-245: LGTM!


247-251: LGTM!


257-260: Remove redundant else clause.

The else clause is unnecessary here because the if condition ends with a return statement, making the else redundant and less readable.

-    } else {
-      addMaterials(response.value.materials)
-    }
+    }
+    addMaterials(response.value.materials);

Likely invalid or redundant comment.

Tools
Biome

[error] 257-257: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8ac2a2b and 9866981.

Files selected for processing (3)
  • packages/plugins/materials/index.js (1 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
  • packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/plugins/materials/index.js
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 49-50: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 261-261: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)


[error] 348-348: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

Additional comments not posted (6)
packages/plugins/materials/src/meta/layout/src/Main.vue (5)

53-53: Verify Initialization and Usage of pluginRegistryData.

Ensure that pluginRegistryData and its properties are correctly initialized and used to avoid potential runtime errors.


55-55: LGTM!

The initialization of onlyShowDefault is appropriate.


57-58: LGTM!

The initialization of activeTabId is appropriate.


62-62: LGTM!

The initialization of tabComponents is appropriate.


Line range hint 227-228: Verify Initialization and Usage of materialState.blocks.

Ensure that materialState.blocks and its properties are correctly initialized and used to avoid potential runtime errors.

packages/plugins/materials/src/composable/useMaterial.js (1)

321-327: Avoid Spread Syntax on Accumulators.

Spread syntax should be avoided on accumulators because it causes a time complexity of O(n^2). Consider using methods such as .splice or .push instead.

-    componentState.componentsMap = appData.componentsMap?.reduce((componentsMap, component) => {
-      if (component.dependencies) {
-        getBlockDeps(component.dependencies)
-      }
-      return { ...componentsMap, [component.componentName]: component }
-    }, {})
+    componentState.componentsMap = {};
+    appData.componentsMap?.forEach((component) => {
+      if (component.dependencies) {
+        getBlockDeps(component.dependencies);
+      }
+      componentState.componentsMap[component.componentName] = component;
+    });

Likely invalid or redundant comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9866981 and 104845b.

Files selected for processing (3)
  • packages/entry/src/hooks.js (3 hunks)
  • packages/plugins/block/src/js/blockSetting.jsx (2 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/entry/src/hooks.js
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 53-54: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 270-270: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)


[error] 353-355: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

packages/plugins/block/src/js/blockSetting.jsx

[error] 490-490: This default parameter should follow the last required parameter or should be a required parameter.

The last required parameter is here:

A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.

(lint/style/useDefaultParameterLast)

Additional comments not posted (1)
packages/plugins/block/src/js/blockSetting.jsx (1)

23-24: LGTM!

The import statement and the usage of getMaterial from useMaterial align with the objective of decoupling material-related resources from useResource.

Also applies to: 54-54

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 104845b and 05ec9dd.

Files selected for processing (1)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 53-54: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


[error] 270-270: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Additional comments not posted (9)
packages/plugins/materials/src/composable/useMaterial.js (9)

68-73: LGTM!


79-87: LGTM!


90-103: LGTM!


152-156: LGTM!


158-158: LGTM!


211-234: LGTM!


236-249: LGTM!


251-253: LGTM!


265-272: Remove redundant else clause.

The else clause is unnecessary because the previous branch ends with a return statement.

-  if (response.status === 'fulfilled' && response.value.materials) {
-    addMaterials(response.value.materials)
-  } else {
-    // handle other cases
-  }
+  if (response.status === 'fulfilled' && response.value.materials) {
+    addMaterials(response.value.materials);
+  }

Likely invalid or redundant comment.

Tools
Biome

[error] 270-270: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 05ec9dd and fbbc05c.

Files selected for processing (1)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
Additional context used
Biome
packages/plugins/materials/src/composable/useMaterial.js

[error] 274-274: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Additional comments not posted (19)
packages/plugins/materials/src/composable/useMaterial.js (19)

1-18: LGTM!


39-51: LGTM!


53-66: LGTM!


72-77: LGTM!


83-91: LGTM!


94-107: LGTM!


156-160: LGTM!


162-162: LGTM!


168-198: LGTM!


205-209: LGTM!


215-238: LGTM!


240-252: LGTM!


255-257: LGTM!


283-301: LGTM!


307-315: LGTM!


317-328: LGTM!


330-341: LGTM!


343-357: LGTM!


269-276: Remove redundant else clause.

The else clause is unnecessary because the previous branch ends with a return statement.

-  materials.forEach((response) => {
-    if (response.status === 'fulfilled' && response.value.materials) {
-      addMaterials(response.value.materials)
-    }
-  })
+  materials.forEach((response) => {
+    if (response.status === 'fulfilled' && response.value.materials) {
+      addMaterials(response.value.materials);
+    }
+  });

Likely invalid or redundant comment.

Tools
Biome

[error] 274-274: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
packages/plugins/block/src/js/blockSetting.jsx (1)

Line range hint 495-495: Consider adjusting the parameter order in function definitions.

The default parameter precedes a required parameter, which can lead to unexpected behavior or errors if not all parameters are provided during a function call. Consider rearranging the parameters to follow best practices.

- function example(defaultParam = 'default', requiredParam) {
+ function example(requiredParam, defaultParam = 'default') {
packages/canvas/container/src/container.js (1)

Line range hint 239-247: Redundant else clause can be omitted.

Following the static analysis tool's suggestion, the else clause here is unnecessary because the preceding branches (if and else-if) cover all conditions that would prevent the else branch from executing.

- else {
-   type = POSITION.BOTTOM;
- }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fbbc05c and 3b337d1.

Files selected for processing (28)
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue (2 hunks)
  • packages/canvas/container/src/CanvasContainer.vue (2 hunks)
  • packages/canvas/container/src/components/CanvasAction.vue (2 hunks)
  • packages/canvas/container/src/components/shortCutPopover.vue (2 hunks)
  • packages/canvas/container/src/container.js (2 hunks)
  • packages/common/component/ConfigItem.vue (2 hunks)
  • packages/common/js/preview.js (2 hunks)
  • packages/configurator/src/container-configurator/ContainerConfigurator.vue (2 hunks)
  • packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue (2 hunks)
  • packages/configurator/src/slot-configurator/SlotConfigurator.vue (2 hunks)
  • packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue (1 hunks)
  • packages/design-core/index.js (1 hunks)
  • packages/plugins/block/src/js/blockSetting.jsx (2 hunks)
  • packages/plugins/materials/src/composable/index.js (2 hunks)
  • packages/plugins/materials/src/composable/useMaterial.js (1 hunks)
  • packages/plugins/materials/src/composable/useResource.js (5 hunks)
  • packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (2 hunks)
  • packages/plugins/materials/src/meta/block/src/BlockList.vue (2 hunks)
  • packages/plugins/materials/src/meta/block/src/Main.vue (3 hunks)
  • packages/plugins/materials/src/meta/component/src/Main.vue (2 hunks)
  • packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
  • packages/plugins/page/src/Main.vue (3 hunks)
  • packages/plugins/tree/src/Main.vue (2 hunks)
  • packages/register/src/hooks.js (3 hunks)
  • packages/settings/events/src/components/BindEvents.vue (2 hunks)
  • packages/settings/props/src/components/inputs/BindFunction.vue (2 hunks)
  • packages/settings/props/src/components/inputs/DraggableOptions.vue (3 hunks)
  • packages/settings/props/src/composable/useProperties.js (2 hunks)
Files skipped from review due to trivial changes (1)
  • packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue
Files skipped from review as they are similar to previous changes (21)
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue
  • packages/canvas/container/src/CanvasContainer.vue
  • packages/canvas/container/src/components/CanvasAction.vue
  • packages/canvas/container/src/components/shortCutPopover.vue
  • packages/common/component/ConfigItem.vue
  • packages/configurator/src/container-configurator/ContainerConfigurator.vue
  • packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue
  • packages/configurator/src/slot-configurator/SlotConfigurator.vue
  • packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue
  • packages/design-core/index.js
  • packages/plugins/materials/src/composable/index.js
  • packages/plugins/materials/src/composable/useResource.js
  • packages/plugins/materials/src/meta/block/src/BlockList.vue
  • packages/plugins/materials/src/meta/block/src/Main.vue
  • packages/plugins/materials/src/meta/component/src/Main.vue
  • packages/plugins/materials/src/meta/layout/src/Main.vue
  • packages/plugins/page/src/Main.vue
  • packages/plugins/tree/src/Main.vue
  • packages/settings/events/src/components/BindEvents.vue
  • packages/settings/props/src/components/inputs/BindFunction.vue
  • packages/settings/props/src/components/inputs/DraggableOptions.vue
Additional context used
Biome
packages/common/js/preview.js

[error] 31-31: Avoid the use of spread (...) syntax on accumulators.

Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.

(lint/performance/noAccumulatingSpread)

packages/settings/props/src/composable/useProperties.js

[error] 219-222: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

packages/plugins/materials/src/composable/useMaterial.js

[error] 274-274: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

packages/plugins/block/src/js/blockSetting.jsx

[error] 495-495: This default parameter should follow the last required parameter or should be a required parameter.

The last required parameter is here:

A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.

(lint/style/useDefaultParameterLast)

packages/canvas/container/src/container.js

[error] 239-247: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

Additional comments not posted (10)
packages/common/js/preview.js (1)

15-15: Correct import of useMaterial.

The import of useMaterial aligns with the PR's objectives to decouple material-related functionalities from useResource.

packages/register/src/hooks.js (2)

21-22: Proper integration of new hooks.

The addition of useMaterial and useCustom hooks aligns with the decoupling efforts and extends the functionality of the hooks system.

Also applies to: 45-45


75-75: Correct definition of useMaterial hook function.

The useMaterial function is defined correctly and follows the established pattern for hook functions in this file.

packages/settings/props/src/composable/useProperties.js (1)

15-15: Correct integration of useMaterial.

The import and usage of useMaterial are correctly implemented, aligning with the decoupling efforts of the PR.

packages/plugins/materials/src/composable/useMaterial.js (2)

115-153: Remove redundant else clause and simplify nested structure.

This suggestion was previously made and is still applicable. Simplifying the nested structure improves readability and maintainability.


274-274: Remove unnecessary else clause.

This suggestion was previously made and is still valid. Removing the unnecessary else clause simplifies the control flow.

Tools
Biome

[error] 274-274: This else clause can be omitted because previous branches break early.

Unsafe fix: Omit the else clause.

(lint/style/noUselessElse)

packages/plugins/block/src/js/blockSetting.jsx (2)

25-25: Import of useMaterial is properly added.

The import of useMaterial is correctly integrated to align with the project's shift from useResource to useMaterial. This change is consistent with the PR's objectives and the broader refactoring efforts across the project.


54-54: Proper usage of useMaterial for accessing getMaterial.

The line correctly uses the useMaterial hook to access the getMaterial method. This is part of the project's initiative to centralize material-related functionalities, enhancing modularity and maintainability.

packages/canvas/container/src/container.js (2)

22-22: Update import to include useMaterial.

The import statement correctly includes useMaterial along with other hooks. This change is necessary for the new functionality introduced in the PR and aligns with the project's direction.


919-920: Correct integration of useMaterial functions in initCanvas.

The use of useMaterial().getConfigureMap() and useMaterial().materialState.thirdPartyDeps in the initCanvas function is appropriate and aligns with the PR's objectives to centralize material-related functionalities.

packages/common/js/preview.js Outdated Show resolved Hide resolved
packages/settings/props/src/composable/useProperties.js Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3b337d1 and 3a3c210.

Files selected for processing (2)
  • packages/common/js/preview.js (2 hunks)
  • packages/settings/props/src/composable/useProperties.js (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/common/js/preview.js
Additional context used
Biome
packages/settings/props/src/composable/useProperties.js

[error] 219-222: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (1)
packages/settings/props/src/composable/useProperties.js (1)

15-15: Update the import statement.

The import of useMaterial replaces useResource. Ensure that useMaterial is correctly used throughout the codebase.

Verification successful

Update the import statement.

The import of useMaterial replaces useResource. The usage of useMaterial has been verified throughout the codebase.

  • packages/settings/props/src/composable/useProperties.js
  • packages/register/src/hooks.js
  • packages/canvas/container/src/CanvasContainer.vue
  • packages/canvas/DesignCanvas/src/DesignCanvas.vue
  • packages/canvas/container/src/container.js
  • packages/canvas/container/src/components/shortCutPopover.vue
  • packages/canvas/container/src/components/CanvasAction.vue
  • packages/common/js/preview.js
  • packages/common/component/ConfigItem.vue
  • packages/configurator/src/slot-configurator/SlotConfigurator.vue
  • packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue
  • packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue
  • packages/configurator/src/container-configurator/ContainerConfigurator.vue
  • packages/toolbars/refresh/src/Main.vue
  • packages/plugins/tree/src/Main.vue
  • packages/plugins/materials/src/composable/useResource.js
  • packages/plugins/materials/src/composable/index.js
  • packages/plugins/materials/src/meta/block/src/Main.vue
  • packages/plugins/materials/src/meta/block/src/BlockList.vue
  • packages/plugins/materials/src/meta/component/src/Main.vue
  • packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue
  • packages/settings/events/src/components/BindEvents.vue
  • packages/settings/props/src/components/inputs/BindFunction.vue
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the usage of `useMaterial` in the codebase.

# Test: Search for the function usage. Expect: Only occurrences of `useMaterial`.
rg --type js --type vue $'useMaterial'

Length of output: 65



Script:

#!/bin/bash
# Description: Verify the usage of `useMaterial` in the codebase.

# Test: Search for the function usage in .js and .vue files. Expect: Only occurrences of `useMaterial`.
rg --glob '*.js' --glob '*.vue' 'useMaterial'

Length of output: 6185

@hexqi hexqi changed the title useResource解耦第一版:将物料相关资源从useResource中剥离 refactor: extract material-related code to useMaterial from useResource Jul 1, 2024
@hexqi hexqi merged commit 63a0dcb into opentiny:refactor/develop Jul 1, 2024
yy-wow pushed a commit to yy-wow/tiny-engine that referenced this pull request Oct 10, 2024
…ce (opentiny#591)

* refactor(material): 将物料(组件,区块)从resource剥离出来

* refactor(useMaterial): useResource拆分

* refactor(useMaterial): 机器人review意见修改

* refactor(useMaterial): review意见修改

* refactor(useMaterials): 更新冲突后的调整,以及增加注释

* refactor(useMaterials): review bot

* refactor(useMaterials): review bot

* fix: sync update

* refactor(useMaterials): review bot

---------

Co-authored-by: chilingling <michaelchiling1024@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants