chore: move upload script to template #1685
Conversation
1. 将同步物料的脚本移动到用户工程以及模板中。（通过 cli 模板可以直接得到 script） 2. 使用 FormData API 替代手动构造 multipart/form-data 请求体；增强错误处理和日志输出；增强功能详细说明 3. 将 `updateTempmlate` 中的 `assert` 语法改为 `with` （Node.js v22 assert 语法已经被废弃）
WalkthroughAdds Logger utilities and new uploadMaterials scripts in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PNPM as pnpm
participant Script as uploadMaterials.mjs
participant FS as FileSystem
participant Logger as Logger
participant Backend as BackendAPI
User->>PNPM: run uploadMaterials (pnpm --filter designer-demo)
PNPM->>Script: invoke script
Script->>FS: read env/.env.local
Script->>Logger: info "env loaded"
Script->>FS: read public/mock/bundle.json
Script->>Logger: info "bundle loaded"
Script->>Logger: info "starting upload"
Script->>Backend: POST /material-center/api/component/bundle/create (multipart/form-data)
alt Success (2xx & success:true)
Backend-->>Script: 200 + JSON { success:true, inserted/updated }
Script->>Logger: success "upload complete"
else Failure (non-OK or success:false)
Backend-->>Script: error / JSON { success:false, message }
Script->>Logger: warn/error "upload failed"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
chilingling
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
designer-demo/scripts/logger.mjs (1)
1-43: LGTM! Clean logger implementation with optional simplification.The Logger class provides a clean interface for colored, timestamped logging. The implementation is straightforward and appropriate for the use case.
Optional simplification at line 21:
- const _logger = console - - return _logger.log(format()) + return console.log(format())packages/engine-cli/template/designer/scripts/uploadMaterials.mjs (1)
35-38: Consider validating bundle.json existence.The script could provide a clearer error message if the bundle.json file doesn't exist before attempting to read it.
const bundlePath = path.join(process.cwd(), './public/mock/bundle.json') logger.info(`Start to read bundle.json file from ${bundlePath}`) + if (!fs.existsSync(bundlePath)) { + logger.error(`Bundle file not found at ${bundlePath}`) + process.exit(1) + } const bundle = fs.readJSONSync(bundlePath)designer-demo/scripts/uploadMaterials.mjs (1)
36-39: Consider validating bundle.json existence.Provide a clearer error if the bundle file is missing.
const bundlePath = path.join(process.cwd(), './public/mock/bundle.json') logger.info(`Start to read bundle.json file from ${bundlePath}`) + if (!fs.existsSync(bundlePath)) { + logger.error(`Bundle file not found at ${bundlePath}`) + process.exit(1) + } const bundle = fs.readJSONSync(bundlePath)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
designer-demo/package.json(2 hunks)designer-demo/scripts/logger.mjs(1 hunks)designer-demo/scripts/uploadMaterials.mjs(1 hunks)package.json(1 hunks)packages/engine-cli/template/designer/package.json(2 hunks)packages/engine-cli/template/designer/scripts/logger.mjs(1 hunks)packages/engine-cli/template/designer/scripts/uploadMaterials.mjs(1 hunks)scripts/updateTemplate.mjs(1 hunks)scripts/uploadMaterials.mjs(0 hunks)
💤 Files with no reviewable changes (1)
- scripts/uploadMaterials.mjs
🧰 Additional context used
🧬 Code graph analysis (2)
packages/engine-cli/template/designer/scripts/logger.mjs (1)
designer-demo/scripts/logger.mjs (5)
format(9-20)colorMap(10-15)time(16-16)colorMsg(17-17)_logger(21-21)
packages/engine-cli/template/designer/scripts/uploadMaterials.mjs (1)
designer-demo/scripts/uploadMaterials.mjs (13)
logger(21-21)logger(59-59)appDirectory(24-24)process(29-29)resolveApp(25-25)pathsDotenv(26-26)bundlePath(36-36)bundle(38-38)jsonBuffer(39-39)requestUrl(41-41)formData(44-44)response(46-49)data(50-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (8)
scripts/updateTemplate.mjs (1)
5-5: LGTM! Correct syntax update for Node.js v22.The migration from
asserttowithfor import attributes aligns with Node.js v22's deprecation of the older syntax.package.json (1)
24-24: LGTM! Script delegation to monorepo package.The update correctly delegates the upload task to the designer-demo package using monorepo filtering, aligning with the PR's goal of moving the script to the template.
packages/engine-cli/template/designer/package.json (2)
10-10: LGTM! Upload script entry added.The new uploadMaterials script entry correctly points to the implementation in the scripts directory.
32-34: LGTM! Dependencies appropriate for upload functionality.The added devDependencies (dotenv, fs-extra, picocolors) are appropriate for environment configuration, file operations, and colored logging in the upload script.
packages/engine-cli/template/designer/scripts/logger.mjs (1)
1-43: LGTM! Intentional duplication for template distribution.This Logger implementation is identical to the one in designer-demo/scripts/logger.mjs. The duplication is intentional since the template needs a standalone copy for distribution to users. Be mindful that changes to the logger should be synchronized between both locations.
designer-demo/package.json (2)
12-12: LGTM! Upload script entry added.The uploadMaterials script correctly points to the new implementation.
32-34: LGTM! Dependencies consistent with template.The devDependencies match the template's requirements and are appropriate for the upload functionality.
designer-demo/scripts/uploadMaterials.mjs (1)
1-62: Note: Intentional duplication with template script.This implementation is nearly identical to packages/engine-cli/template/designer/scripts/uploadMaterials.mjs. The duplication is intentional for template distribution, but changes should be synchronized between both files.
| try { | ||
| const formData = new FormData() | ||
| formData.append('file', new Blob([jsonBuffer], { type: 'application/json'}), 'bundle.json') | ||
| const response = await fetch(requestUrl, { | ||
| method: 'POST', | ||
| body: formData | ||
| }) | ||
| const data = await response.json() | ||
| logger.success('File uploaded successfully:', data) | ||
| } catch (error) { | ||
| logger.error('Error uploading file:', error instanceof Error ? error.message : String(error)) | ||
| } |
There was a problem hiding this comment.
Validate HTTP response status before parsing.
The script doesn't check if the request succeeded before parsing JSON. Non-2xx responses may cause parsing failures or silent errors.
Apply this diff:
const response = await fetch(requestUrl, {
method: 'POST',
body: formData
})
+ if (!response.ok) {
+ const errorText = await response.text()
+ throw new Error(`Upload failed with status ${response.status}: ${errorText}`)
+ }
const data = await response.json()
- logger.success('File uploaded successfully:', data)
+ logger.success(`File uploaded successfully: ${JSON.stringify(data)}`)
} catch (error) {🤖 Prompt for AI Agents
In designer-demo/scripts/uploadMaterials.mjs around lines 43 to 54, the code
parses response.json() without checking HTTP status which can throw or hide
errors on non-2xx responses; update the try block to first check response.ok (or
response.status) after fetch, and if not ok, read the response body (text or
json) and throw or log a descriptive error including status and body; only call
response.json() when response.ok is true and then proceed to logger.success with
the parsed data.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
designer-demo/scripts/uploadMaterials.mjs (2)
18-18: Remove or clarify the@returnstag.The
mainfunction doesn't return a value, so the@returnstag should be removed or updated to indicate it returnsPromise<void>.Apply this diff:
* 2. 本地已经将 bundle.json 文件进行修改,但是出码仍然不正确。 - * @returns + * @returns {Promise<void>} */
56-56: Consider stringifying the response data for clearer logs.The success log directly outputs the data object. Stringifying it would ensure consistent, readable output regardless of the logger implementation.
Apply this diff:
- logger.success('File uploaded successfully:', data) + logger.success(`File uploaded successfully: ${JSON.stringify(data)}`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
designer-demo/scripts/uploadMaterials.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (2)
designer-demo/scripts/uploadMaterials.mjs (2)
31-34: Validation now exits with error code - previous issue resolved.The code now correctly exits with status code 1 when
backend_urlis missing, which will properly signal failure in CI/CD workflows.
51-54: HTTP response validation implemented - previous issue resolved.The code now correctly validates
response.okand throws a descriptive error with status and body text, which addresses the previous review concern.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
designer-demo/scripts/logger.mjs(1 hunks)designer-demo/scripts/uploadMaterials.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- designer-demo/scripts/uploadMaterials.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
designer-demo/scripts/logger.mjs (1)
packages/engine-cli/template/designer/scripts/logger.mjs (2)
time(16-16)colorMap(10-15)
🪛 GitHub Actions: Push And Create PR Check
designer-demo/scripts/logger.mjs
[error] 9-9: ESLint: A 'require' style import is forbidden. @typescript-eslint/no-require-imports
🪛 GitHub Check: push-check
designer-demo/scripts/logger.mjs
[failure] 47-47:
Unexpected console statement
[failure] 39-39:
Unexpected console statement
[failure] 30-30:
A require() style import is forbidden
[failure] 12-12:
Unexpected console statement
[failure] 9-9:
A require() style import is forbidden
🔇 Additional comments (2)
designer-demo/scripts/logger.mjs (2)
1-5: LGTM: Constructor is well-structured.The constructor properly initializes the command and color support detection.
51-65: LGTM: Public API methods are clean.The delegation pattern with rest parameters is appropriate and maintains a consistent interface.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
designer-demo/scripts/logger.mjs (1)
1-88: Duplicate implementation has same critical issues.This Logger implementation is nearly identical to the template version at
packages/engine-cli/template/designer/scripts/logger.mjs, which is good for consistency. However, it inherits the same critical race condition whereinitColors()is called asynchronously from the constructor withoutawait(line 22), and potentially imports the wrong color package (line 27).Apply the same fixes suggested for the template version to resolve:
- The race condition in async initialization
- The package mismatch (if
picocolorsis the correct dependency)Keeping both files synchronized will prevent future divergence issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
bash.exe.stackdump(1 hunks)designer-demo/scripts/logger.mjs(1 hunks)designer-demo/scripts/uploadMaterials.mjs(1 hunks)packages/engine-cli/template/designer/scripts/logger.mjs(1 hunks)packages/engine-cli/template/designer/scripts/uploadMaterials.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- bash.exe.stackdump
🚧 Files skipped from review as they are similar to previous changes (2)
- designer-demo/scripts/uploadMaterials.mjs
- packages/engine-cli/template/designer/scripts/uploadMaterials.mjs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.
Applied to files:
designer-demo/scripts/logger.mjs
📚 Learning: 2025-01-14T06:55:14.457Z
Learnt from: rhlin
Repo: opentiny/tiny-engine PR: 1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-01-14T06:55:14.457Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.
Applied to files:
designer-demo/scripts/logger.mjs
📚 Learning: 2025-07-03T09:22:59.512Z
Learnt from: hexqi
Repo: opentiny/tiny-engine PR: 1501
File: mockServer/src/tool/Common.js:79-82
Timestamp: 2025-07-03T09:22:59.512Z
Learning: In the tiny-engine project, the mockServer code uses ES6 import syntax but is compiled to CommonJS output. This means CommonJS globals like `__dirname` are available at runtime, while ES6 module-specific features like `import.meta` would cause runtime errors.
Applied to files:
designer-demo/scripts/logger.mjs
🧬 Code graph analysis (1)
packages/engine-cli/template/designer/scripts/logger.mjs (1)
designer-demo/scripts/logger.mjs (10)
log(1-7)warn(9-15)colorsModule(27-27)time(37-37)prefix(38-38)message(39-44)outputFn(46-46)colorMap(49-54)coloredType(55-57)emojiMap(61-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/engine-cli/template/designer/scripts/logger.mjs (3)
1-15: LGTM!The
logandwarnhelper functions correctly route output to stdout and stderr respectively, with appropriate JSON formatting for object arguments.
36-69: Output logic looks correct.The method properly selects stdout vs stderr based on log level, formats messages consistently, and handles both colored and emoji-based output. Once the race condition in the constructor is resolved, this will function as intended.
71-88: LGTM!The public API methods correctly delegate to
output()with the appropriate log level, and the default export is properly declared.
| class Logger { | ||
| constructor(command = 'default') { | ||
| this.command = command | ||
| this.colors = null | ||
| this.hasColors = false | ||
| this.initColors() | ||
| } |
There was a problem hiding this comment.
Critical race condition: async initialization in constructor.
initColors() is async but called without await from the constructor. This creates a race condition where output() calls made immediately after instantiation will always see hasColors = false and colors = null, causing colorization to be skipped even when the package is available.
Consider one of these approaches:
Option 1: Static factory method (recommended)
- constructor(command = 'default') {
+ static async create(command = 'default') {
+ const logger = new Logger(command);
+ await logger.initColors();
+ return logger;
+ }
+
+ constructor(command = 'default') {
this.command = command
this.colors = null
this.hasColors = false
- this.initColors()
}Option 2: Lazy initialization on first use
constructor(command = 'default') {
this.command = command
this.colors = null
this.hasColors = false
- this.initColors()
+ this.colorInitPromise = this.initColors()
}
- output(type, ...args) {
+ async output(type, ...args) {
+ await this.colorInitPromise
const time = new Date().toLocaleTimeString()
// ... rest of method
}📝 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.
| class Logger { | |
| constructor(command = 'default') { | |
| this.command = command | |
| this.colors = null | |
| this.hasColors = false | |
| this.initColors() | |
| } | |
| class Logger { | |
| static async create(command = 'default') { | |
| const logger = new Logger(command); | |
| await logger.initColors(); | |
| return logger; | |
| } | |
| constructor(command = 'default') { | |
| this.command = command | |
| this.colors = null | |
| this.hasColors = false | |
| } |
| async initColors() { | ||
| try { | ||
| const colorsModule = await import('colors') | ||
| this.colors = colorsModule.default || colorsModule | ||
| this.hasColors = true | ||
| } catch (err) { | ||
| warn('colors package not found, using basic logging') | ||
| this.hasColors = false | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify correct color package is imported.
Based on past review comments, the PR adds picocolors to devDependencies, but this code imports colors. If the dependency mismatch is real, the import will always fail and colors will never be available.
Run this script to verify which color package is actually in the dependencies:
🏁 Script executed:
#!/bin/bash
# Check which color packages are in package.json files
echo "=== Template designer package.json ==="
fd -t f '^package.json$' packages/engine-cli/template/designer --exec jq -r 'select(.devDependencies) | .devDependencies | to_entries | .[] | select(.key | contains("color")) | "\(.key): \(.value)"' {}
echo -e "\n=== Designer-demo package.json ==="
fd -t f '^package.json$' designer-demo --exec jq -r 'select(.devDependencies) | .devDependencies | to_entries | .[] | select(.key | contains("color")) | "\(.key): \(.value)"' {}
echo -e "\n=== Root package.json ==="
jq -r 'select(.devDependencies) | .devDependencies | to_entries | .[] | select(.key | contains("color")) | "\(.key): \(.value)"' package.jsonLength of output: 158
Fix incorrect package import in logger.mjs.
The code imports colors (line 27) but the available dependency is picocolors. All package.json files in the repository specify only picocolors as available. Change the import statement to use the correct package:
const colorsModule = await import('picocolors')🤖 Prompt for AI Agents
In packages/engine-cli/template/designer/scripts/logger.mjs around lines 25 to
34, the code attempts to dynamically import the non-existent 'colors' package;
replace that import with the correct package name 'picocolors' so it reads an
import for 'picocolors' instead, then continue to assign this.colors =
colorsModule.default || colorsModule and leave the existing hasColors/try-catch
behavior unchanged.
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
updateTempmlate中的assert语法改为with（Node.js v22 assert 语法已经被废弃）What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Chores