Skip to content

feat:add component and componentLibrary#29

Closed
zhangjuncao wants to merge 3 commits intomainfrom
zz
Closed

feat:add component and componentLibrary#29
zhangjuncao wants to merge 3 commits intomainfrom
zz

Conversation

@zhangjuncao
Copy link
Collaborator

@zhangjuncao zhangjuncao commented Apr 15, 2025

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

  • New Features

    • Introduced API endpoints and backend services for managing component libraries and user components, including creation, update, deletion, and querying.
    • Added support for uploading and processing user component bundles.
  • Improvements

    • Enhanced application schema to include component library package details.
    • Updated enum and configuration for improved AI model integration and CORS support.
  • Configuration

    • Changed default development server and data center ports.
    • Updated AI model endpoint URLs.
  • Bug Fixes

    • Improved error handling and validation for uploaded files.

@coderabbitai
Copy link

coderabbitai bot commented Apr 15, 2025

Walkthrough

This update introduces new controllers and services to manage component libraries and user components within a material center context. It adds CRUD endpoints and logic for component libraries, including bulk and bundle creation for user components with robust file handling and validation. The routing configuration is extended to expose these new API endpoints. Additionally, configuration files are updated for CORS support, a new development server port, and changes to AI model endpoints. Enum values and default service ports are also updated to reflect new requirements.

Changes

File(s) Change Summary
app/controller/material-center/ComponentLibrary.ts
app/controller/material-center/UserComponents.ts
Added new controller classes for component library and user component management, including CRUD and bundle creation endpoints.
app/service/material-center/ComponentLibrary.ts
app/service/material-center/UserComponents.ts
Introduced service classes for handling CRUD operations, bulk creation, bundle parsing, file stream validation, and persistence logic for component libraries and user components. Implements robust error handling and utility functions.
app/router/materialCenter/base.ts Registered new API routes for component library CRUD operations and user component bundle creation.
app/service/app-center/appSchema.ts Extended schema service to fetch and expose component library package data, adding a new method and updating metadata handling.
app/lib/enum.ts Updated the ERNIE_BOT_TURBO enum value from 'ERNIE-Bot-turbo' to 'ERNIE-4.0-8K'.
config/config.default.ts Added CORS configuration and updated the AI model endpoint for ERNIE_BOT_TURBO.
config/config.local.ts Changed the default data center host port from 1337 to 1356.
package.json Changed the development server port in the dev script from 7011 to 7020.
.vscode/settings.json Added a newline character at the end of the file.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Router
    participant ComponentController
    participant ComponentLibraryService
    participant UserComponentController
    participant UserComponentService

    Client->>Router: POST /component-library
    Router->>ComponentController: create()
    ComponentController->>ComponentLibraryService: create()
    ComponentLibraryService-->>ComponentController: result
    ComponentController-->>Client: response

    Client->>Router: POST /component/bundle/create
    Router->>UserComponentController: bundleCreate()
    UserComponentController->>UserComponentService: bundleCreate(fileStream)
    UserComponentService->>UserComponentService: validate & parse file
    UserComponentService->>UserComponentService: bulkComponentCreate()
    UserComponentService-->>UserComponentController: result
    UserComponentController-->>Client: response
Loading

Possibly related PRs

Poem

Hopping through code with nimble delight,
New controllers and services take flight!
Libraries and bundles, all neat and new,
Routes and configs get a fresh review.
With ports and enums now aligned just right,
This bunny celebrates a productive night!
🐇✨

Tip

⚡💬 Agentic Chat (Pro Plan, General Availability)
  • We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 using 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 generate docstrings to generate docstrings for this 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.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration 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

@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: 14

🧹 Nitpick comments (6)
package.json (1)

19-19: Ensure port consistency between dev and debug modes

The development server port has been changed to 7020, but the debug port on line 20 still uses 7011. Consider whether these should be aligned for consistency.

  "dev": "egg-bin dev --port 7020",
- "debug": "egg-bin debug --port 7011",
+ "debug": "egg-bin debug --port 7020",
app/router/materialCenter/base.ts (1)

73-73: Format consistently with other routes

Add space after the comma for readability and consistency with codebase standards.

-subRouter.post('/component/bundle/create',controller.materialCenter.userComponents.bundleCreate);
+subRouter.post('/component/bundle/create', controller.materialCenter.userComponents.bundleCreate);
app/controller/material-center/ComponentLibrary.ts (2)

21-24: Consider adding input validation and error handling.

Currently, the create method directly consumes the request body and delegates creation to the service. For robust APIs, consider validating the input (e.g., using this.ctx.validate(...)) and capturing any errors thrown by the service to provide a safe, informative response.


49-52: Add error handling for the delete operation.

Similar to the update method, the delete method directly destructures id and calls the service. Adding suppression for missing or invalid id parameters and gracefully handling service errors can strengthen the controller’s reliability.

app/service/material-center/UserComponents.ts (2)

85-122: Review the utility of componentLibraryListResult and handle array property merging carefully.

  1. let componentLibraryListResult: any = []; is assigned but never returned or used later. Remove it if not needed.
  2. Converting component.component arrays to a comma-separated string (lines 92-94) might cause ambiguity or data loss. Consider storing them as arrays in the database or normalizing their structure at creation time.
🧰 Tools
🪛 Biome (1.9.4)

[error] 103-107: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


177-178: Ensure snippets is defined before iterating.

Since you already check snippets != null && snippets.length != 0, optional chaining might reduce nesting while avoiding runtime errors. For instance:

if (snippets?.length) {
  snippets.forEach((snippetItem) => {
    // ...
  });
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 178-178: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cce0e5b and 1c07a83.

📒 Files selected for processing (11)
  • .vscode/settings.json (1 hunks)
  • app/controller/material-center/ComponentLibrary.ts (1 hunks)
  • app/controller/material-center/UserComponents.ts (1 hunks)
  • app/lib/enum.ts (1 hunks)
  • app/router/materialCenter/base.ts (1 hunks)
  • app/service/app-center/appSchema.ts (2 hunks)
  • app/service/material-center/ComponentLibrary.ts (1 hunks)
  • app/service/material-center/UserComponents.ts (1 hunks)
  • config/config.default.ts (2 hunks)
  • config/config.local.ts (1 hunks)
  • package.json (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/controller/material-center/ComponentLibrary.ts (1)
app/lib/interface.ts (1)
  • I_CreateComponentLibrary (445-460)
🪛 Biome (1.9.4)
app/service/material-center/UserComponents.ts

[error] 103-107: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 178-178: Unsafe usage of optional chaining.

If it short-circuits with 'undefined' the evaluation will throw TypeError here:

(lint/correctness/noUnsafeOptionalChaining)

🔇 Additional comments (6)
.vscode/settings.json (1)

6-6: Addition of newline at end of file is a good practice.

Adding a newline at the end of the file improves POSIX compatibility and prevents potential editor warnings. No action needed.

app/lib/enum.ts (1)

292-292:

Details

✅ Verification successful

Update model name to match API endpoint changes

The ERNIE_BOT_TURBO model name has been updated from 'ERNIE-Bot-turbo' to 'ERNIE-4.0-8K'. This aligns with the updated API endpoint mentioned in the summary, but we should verify that all references to this model are updated consistently.


🏁 Script executed:

#!/bin/bash
# Check if there are any remaining references to the old model name
rg "ERNIE-Bot-turbo" --type ts --type js

# Check if the model is used consistently in configurations and API calls
rg -A 3 "ERNIE_BOT_TURBO" --type ts --type js

Length of output: 1061


Consistent usage of updated model name verified

All instances of the model now reference "ERNIE-4.0-8K", and no lingering references to the old "ERNIE-Bot-turbo" were found. The enumeration in app/lib/enum.ts and its usage in both config/config.default.ts and app/service/app-center/aiChat.ts are updated accordingly.

app/service/app-center/appSchema.ts (2)

25-26: LGTM - New package information map entry

The execMap has been correctly extended with a new 'packages' entry that maps to the getPackages method.


57-63:

Details

❓ Verification inconclusive

Ensure error handling when executing schema functions

With the addition of the new 'packages' entry to execMap, verify that error handling in this loop is sufficient for the new method. Currently, if a function returns an error, it returns early, which might prevent processing other schema components.

Consider a more robust approach where errors in one function don't prevent the entire schema from being built. This is related to the implementation of the new getPackages method.


🏁 Script executed:

#!/bin/bash
# Check how errors are handled in the AppSchema class
rg -A 3 -B 3 "error" app/service/app-center/appSchema.ts

Length of output: 246


Revisit Error-Handling Strategy in Schema Function Execution

The loop at lines 57–63 in app/service/app-center/appSchema.ts returns immediately when any function reports an error. With the newly added 'packages' entry (i.e., the getPackages method) in the execMap, please verify whether this early return is the intended behavior. If the goal is to build the complete schema even when one function fails, consider altering the current approach to aggregate errors (or at least log them) rather than stopping execution immediately.

  • File: app/service/app-center/appSchema.ts (lines 57–63)
  • Issue: Early exit on error may prevent other valid schema components from being processed, which is particularly relevant for the new 'packages' functionality.
  • Suggestion: Review whether the early return is the best strategy or if a more robust error-handling mechanism should be implemented to allow partial schema construction while properly managing errors.
app/controller/material-center/ComponentLibrary.ts (1)

38-42: Verify that id is present and properly validated.

In the update method, id is directly destructured from this.ctx.params. Ensure the route is configured to pass id and consider validating id before invoking the service to prevent unexpected behavior.

app/service/material-center/UserComponents.ts (1)

103-104: Use optional chaining to simplify property checks.

Instead of:

if (component.npm && component.npm.package) {
  packageName = component.npm.package;
}

you could write:

packageName = component.npm?.package || '';
🧰 Tools
🪛 Biome (1.9.4)

[error] 103-107: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

const config: PowerPartial<EggAppConfig> = {};
config.dataCenter = {
host: process.env.DATA_CENTER_URL || 'http://localhost:1337',
host: process.env.DATA_CENTER_URL || 'http://localhost:1356',
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify port consistency across the system

The default port for the data center host has been changed from 1337 to 1356. This aligns with the new material center services for component libraries, but it's important to ensure this port change is consistently reflected in other parts of the system.


🏁 Script executed:

#!/bin/bash
# Search for any other occurrences of the old port (1337) in the codebase
rg -l "1337" --type ts --type js

Length of output: 58


Action Item: Align Port Configuration Across All Environments

The new default port (1356) in config/config.local.ts looks correct for the updated material center services. However, our codebase search shows that config/config.unittest.ts still references the old port (1337). Please verify whether this discrepancy is intentional for unit tests and, if not, update config/config.unittest.ts to use the new port (1356) to maintain consistency across the system.

Comment on lines +83 to +85
const componentLibraryData = await this.service.materialCenter.componentLibrary.list();
this.meta = metaData.data;
this.meta.componentLibrary = componentLibraryData.data;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for component library fetch

The setMeta method now fetches component library data, but there's no error handling for cases where the fetch operation fails. This could lead to undefined behavior if the service is unavailable.

private async setMeta(query?): Promise<I_Response> {
  const metaData: I_Response = await this.service.appCenter.apps.schemaMeta(this.appId, query);
-  const componentLibraryData = await this.service.materialCenter.componentLibrary.list();
+  let componentLibraryData = { data: [] };
+  try {
+    componentLibraryData = await this.service.materialCenter.componentLibrary.list();
+  } catch (error) {
+    this.ctx.logger.error('Failed to fetch component library data', error);
+  }
  this.meta = metaData.data;
  this.meta.componentLibrary = componentLibraryData.data;
  return metaData;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const componentLibraryData = await this.service.materialCenter.componentLibrary.list();
this.meta = metaData.data;
this.meta.componentLibrary = componentLibraryData.data;
private async setMeta(query?): Promise<I_Response> {
const metaData: I_Response = await this.service.appCenter.apps.schemaMeta(this.appId, query);
let componentLibraryData = { data: [] };
try {
componentLibraryData = await this.service.materialCenter.componentLibrary.list();
} catch (error) {
this.ctx.logger.error('Failed to fetch component library data', error);
}
this.meta = metaData.data;
this.meta.componentLibrary = componentLibraryData.data;
return metaData;
}

Comment on lines +89 to +100
private getPackages() {
const {componentLibrary} = this.meta;
const packages = componentLibrary.map((item) => ({
name: item.name,
package: item.packageName,
version: item.version,
script: item.script,
css: item.css,
others: item.others
}));
return this.ctx.helper.getResponseData(packages);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for component library data

The getPackages method assumes that componentLibrary always exists and has the expected structure. Add validation to handle cases where the data might be missing or malformed.

private getPackages() {
-  const {componentLibrary} = this.meta;
+  const componentLibrary = this.meta.componentLibrary || [];
  const packages = componentLibrary.map((item) => ({
    name: item.name,
    package: item.packageName,
    version: item.version,
    script: item.script,
    css: item.css,
    others: item.others
  }));
  return this.ctx.helper.getResponseData(packages);
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private getPackages() {
const {componentLibrary} = this.meta;
const packages = componentLibrary.map((item) => ({
name: item.name,
package: item.packageName,
version: item.version,
script: item.script,
css: item.css,
others: item.others
}));
return this.ctx.helper.getResponseData(packages);
}
private getPackages() {
- const {componentLibrary} = this.meta;
+ const componentLibrary = this.meta.componentLibrary || [];
const packages = componentLibrary.map((item) => ({
name: item.name,
package: item.packageName,
version: item.version,
script: item.script,
css: item.css,
others: item.others
}));
return this.ctx.helper.getResponseData(packages);
}

Comment on lines +65 to +69
//组件库
subRouter.post('/component-library/update/:id',controller.materialCenter.componentLibrary.update);
subRouter.post('/component-library/create',controller.materialCenter.componentLibrary.create);
subRouter.delete('/component-library/delete/:id',controller.materialCenter.componentLibrary.delete);
subRouter.get('/component-library/find',controller.materialCenter.componentLibrary.find);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

API Design: Consider using correct RESTful HTTP methods

The component library endpoints follow RESTful naming but are inconsistent with their HTTP methods. For example, the update endpoint uses POST instead of PUT, while delete correctly uses DELETE.

Consider adjusting the router methods to align with RESTful conventions:

-subRouter.post('/component-library/update/:id',controller.materialCenter.componentLibrary.update);
+subRouter.put('/component-library/update/:id',controller.materialCenter.componentLibrary.update);

Also, add spacing after commas for consistency:

-subRouter.post('/component-library/update/:id',controller.materialCenter.componentLibrary.update);
+subRouter.put('/component-library/update/:id', controller.materialCenter.componentLibrary.update);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
//组件库
subRouter.post('/component-library/update/:id',controller.materialCenter.componentLibrary.update);
subRouter.post('/component-library/create',controller.materialCenter.componentLibrary.create);
subRouter.delete('/component-library/delete/:id',controller.materialCenter.componentLibrary.delete);
subRouter.get('/component-library/find',controller.materialCenter.componentLibrary.find);
//组件库
- subRouter.post('/component-library/update/:id',controller.materialCenter.componentLibrary.update);
+ subRouter.put('/component-library/update/:id', controller.materialCenter.componentLibrary.update);
subRouter.post('/component-library/create',controller.materialCenter.componentLibrary.create);
subRouter.delete('/component-library/delete/:id',controller.materialCenter.componentLibrary.delete);
subRouter.get('/component-library/find',controller.materialCenter.componentLibrary.find);

Comment on lines +77 to +81
config.cors = {
origin: '*', //允许所有来源的请求
allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH',
credentials: true, // 如果需要支持带凭证的跨域请求,则设置为true
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security risk: Overly permissive CORS configuration

The current CORS configuration allows requests from any origin (*) with credentials enabled. This is a significant security risk in production environments as it permits any website to make authenticated requests to your API.

Consider implementing a more restrictive CORS policy:

config.cors = {
-  origin: '*', //允许所有来源的请求
+  origin: process.env.NODE_ENV === 'development' ? '*' : [
+    'https://your-trusted-domain.com',
+    /\.your-domain\.com$/
+  ], // Only allow specific origins in production
  allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH',
  credentials: true, // 如果需要支持带凭证的跨域请求,则设置为true
}

Note: According to CORS specifications, you cannot use credentials: true with origin: '*'. If credentials are enabled, specific origins must be provided.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
config.cors = {
origin: '*', //允许所有来源的请求
allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH',
credentials: true, // 如果需要支持带凭证的跨域请求,则设置为true
}
config.cors = {
- origin: '*', //允许所有来源的请求
+ origin: process.env.NODE_ENV === 'development' ? '*' : [
+ 'https://your-trusted-domain.com',
+ /\.your-domain\.com$/
+ ], // Only allow specific origins in production
allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH',
credentials: true, // 如果需要支持带凭证的跨域请求,则设置为true
}

Comment on lines +27 to +32
async find(param){
const query = qs.stringify(param);
return this.query({
url: `component-library?${query}`
});
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety with proper interfaces

The find method lacks type definitions for its parameters and return value, which reduces type safety.

Define an interface for the find parameters and specify the return type:

+interface FindParams {
+  name?: string;
+  description?: string;
+  [key: string]: any;
+}

-async find(param){
+async find(param: FindParams): Promise<any> {
  const query = qs.stringify(param);
  return this.query({
    url: `component-library?${query}`
  });
}

Comment on lines +38 to +45
async update(param) {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add type safety to the update method

Similar to the find method, the update method also lacks proper type definitions.

+interface UpdateComponentLibrary extends I_CreateComponentLibrary {
+  id: string | number;
+}

-async update(param) {
+async update(param: UpdateComponentLibrary): Promise<any> {
  const { id } = param;
  return this.query({
    url: `component-library/${id}`,
    method: E_Method.Put,
    data: param
  });
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async update(param) {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
}
interface UpdateComponentLibrary extends I_CreateComponentLibrary {
id: string | number;
}
async update(param: UpdateComponentLibrary): Promise<any> {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
}

Comment on lines +17 to +53
export default class ComponentLibrary extends DataService{

async create(param: I_CreateComponentLibrary) {
return this.query({
url: 'component-library' ,
method: E_Method.Post,
data: param
});
}

async find(param){
const query = qs.stringify(param);
return this.query({
url: `component-library?${query}`
});
}

async list() {
return this.query({ url: 'component-library' });
}

async update(param) {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
}

delete({ id }){
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling to service methods

None of the methods include error handling, which is essential for robust API operations.

Add try-catch blocks to handle potential errors:

async create(param: I_CreateComponentLibrary) {
+  try {
    return this.query({ 
      url: 'component-library' ,
      method: E_Method.Post,
      data: param
    });
+  } catch (error) {
+    this.ctx.logger.error('[ComponentLibrary] create error:', error);
+    throw error;
+  }
}

Apply similar error handling to all other methods in the class.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default class ComponentLibrary extends DataService{
async create(param: I_CreateComponentLibrary) {
return this.query({
url: 'component-library' ,
method: E_Method.Post,
data: param
});
}
async find(param){
const query = qs.stringify(param);
return this.query({
url: `component-library?${query}`
});
}
async list() {
return this.query({ url: 'component-library' });
}
async update(param) {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
}
delete({ id }){
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
}
}
export default class ComponentLibrary extends DataService {
async create(param: I_CreateComponentLibrary) {
try {
return this.query({
url: 'component-library',
method: E_Method.Post,
data: param
});
} catch (error) {
this.ctx.logger.error('[ComponentLibrary] create error:', error);
throw error;
}
}
async find(param) {
try {
const query = qs.stringify(param);
return this.query({
url: `component-library?${query}`
});
} catch (error) {
this.ctx.logger.error('[ComponentLibrary] find error:', error);
throw error;
}
}
async list() {
try {
return this.query({ url: 'component-library' });
} catch (error) {
this.ctx.logger.error('[ComponentLibrary] list error:', error);
throw error;
}
}
async update(param) {
try {
const { id } = param;
return this.query({
url: `component-library/${id}`,
method: E_Method.Put,
data: param
});
} catch (error) {
this.ctx.logger.error('[ComponentLibrary] update error:', error);
throw error;
}
}
delete({ id }) {
try {
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
} catch (error) {
this.ctx.logger.error('[ComponentLibrary] delete error:', error);
throw error;
}
}
}

Comment on lines +47 to +52
delete({ id }){
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use consistent method signatures and add async keyword

The delete method signature is inconsistent with other methods and missing the async keyword.

-delete({ id }){
+async delete({ id }: { id: string | number }): Promise<any> {
  return this.query({
    url: `component-library/${id}`,
    method: E_Method.Delete
  });
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
delete({ id }){
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
}
async delete({ id }: { id: string | number }): Promise<any> {
return this.query({
url: `component-library/${id}`,
method: E_Method.Delete
});
}

Comment on lines +58 to +77
packageList.forEach(async (componentLibrary) => {
// 查询是否存在组件库
const paramComponentLibrary = {
name: componentLibrary.name,
version: componentLibrary.version
}
const componentLibraryList = await this.service.materialCenter.componentLibrary.find(paramComponentLibrary);
componentLibrary.packageName = componentLibrary.package;
componentLibrary.framework = 'Vue';
componentLibrary.isOfficial = true;
componentLibrary.isDefault = true;
if(componentLibraryList.data.length) {
// 修改组件库
componentLibrary.id = componentLibraryList.data[0].id;
await this.service.materialCenter.componentLibrary.update(componentLibrary);
}else {
// 新增组件库
await this.service.materialCenter.componentLibrary.create(componentLibrary);
}
})
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid using Array.forEach with async operations.

Here, packageList.forEach(async (componentLibrary) => { ... }) doesn’t wait for async calls to finish. Use await Promise.all(...) or a standard for ... of loop to ensure that all component library updates complete before proceeding. For example:

-    packageList.forEach(async (componentLibrary) => {
+    await Promise.all(packageList.map(async (componentLibrary) => {
       // ...
-    })
+    }))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
packageList.forEach(async (componentLibrary) => {
// 查询是否存在组件库
const paramComponentLibrary = {
name: componentLibrary.name,
version: componentLibrary.version
}
const componentLibraryList = await this.service.materialCenter.componentLibrary.find(paramComponentLibrary);
componentLibrary.packageName = componentLibrary.package;
componentLibrary.framework = 'Vue';
componentLibrary.isOfficial = true;
componentLibrary.isDefault = true;
if(componentLibraryList.data.length) {
// 修改组件库
componentLibrary.id = componentLibraryList.data[0].id;
await this.service.materialCenter.componentLibrary.update(componentLibrary);
}else {
// 新增组件库
await this.service.materialCenter.componentLibrary.create(componentLibrary);
}
})
await Promise.all(packageList.map(async (componentLibrary) => {
// 查询是否存在组件库
const paramComponentLibrary = {
name: componentLibrary.name,
version: componentLibrary.version
}
const componentLibraryList = await this.service.materialCenter.componentLibrary.find(paramComponentLibrary);
componentLibrary.packageName = componentLibrary.package;
componentLibrary.framework = 'Vue';
componentLibrary.isOfficial = true;
componentLibrary.isDefault = true;
if(componentLibraryList.data.length) {
// 修改组件库
componentLibrary.id = componentLibraryList.data[0].id;
await this.service.materialCenter.componentLibrary.update(componentLibrary);
} else {
// 新增组件库
await this.service.materialCenter.componentLibrary.create(componentLibrary);
}
}))

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.

1 participant

Comments