fix: genCode favicon can't work on node runtime #1000
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/vue-generator/src/templates/vue-template/index.jsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/vue-generator/.eslintrc.cjs
WalkthroughThe pull request introduces environment-aware base64 decoding in the Vue generator template. A new constant Changes
Sequence DiagramsequenceDiagram
participant Code as Generator Code
participant Browser as Browser Environment
participant Node as Node.js Environment
Code->>Code: Check isBrowser constant
alt Is Browser
Code->>Browser: Use window.atob()
else Is Node.js
Code->>Node: Use Buffer.from()
end
Code->>Code: Decode base64 data
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/vue-generator/test/testcases/generator/generateApp.test.js (1)
28-43: Robust file content handling implementation!The changes properly handle both text and binary file content, with appropriate type checking and conversion. Consider adding try-catch blocks for file operations to handle potential I/O errors gracefully.
for (const { fileName, path: filePath, fileContent } of genResult) { + try { fs.mkdirSync(path.resolve(__dirname, `./result/appdemo01/${filePath}`), { recursive: true }) if (typeof fileContent === 'string') { fs.writeFileSync( path.resolve(__dirname, `./result/appdemo01/${filePath}/${fileName}`), fileContent.replace(/\r?\n/g, '\r\n') ) } else if (fileContent instanceof Blob) { const arrayBuffer = await fileContent.arrayBuffer() const buffer = Buffer.from(arrayBuffer) fs.writeFileSync(path.resolve(__dirname, `./result/appdemo01/${filePath}/${fileName}`), buffer) } + } catch (error) { + console.error(`Failed to write file ${fileName}:`, error) + throw error + } }packages/vue-generator/src/templates/vue-template/index.js (2)
59-63: Optimize Node.js base64 decoding pathWhile the environment-aware implementation is correct, the Node.js path can be simplified by directly using the buffer for the Uint8Array.
if (isBrowser) { raw = window.atob(arr[1]) } else { - raw = Buffer.from(arr[1], 'base64').toString('binary') + const buffer = Buffer.from(arr[1], 'base64') + return new Blob([buffer], { type: mime }) }
169-180: Enhance error logging for favicon generationThe error handling is good, but the error message could be more descriptive to aid debugging.
} catch (error) { // eslint-disable-next-line no-console - console.error('generate favicon.ico error', error) + console.error('Failed to generate favicon.ico: Unable to convert base64 to blob', error) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/vue-generator/test/testcases/generator/expected/appdemo01/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (2)
packages/vue-generator/src/templates/vue-template/index.js(3 hunks)packages/vue-generator/test/testcases/generator/generateApp.test.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (2)
packages/vue-generator/test/testcases/generator/generateApp.test.js (1)
9-18: Well-implemented mock for favicon!The mock correctly simulates the favicon import by reading the actual file and converting it to a base64 data URL, which matches the expected format for favicon handling.
packages/vue-generator/src/templates/vue-template/index.js (1)
36-36: Robust browser environment detection!The implementation follows best practices by checking both
windowanddocumentexistence.
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
【问题描述】
【问题分析】
因为 vitest 测试模式下，import 进来的文件不是 base64 字符串（编译后是 base64 字符串）
导致单元测试无法正确读取二进制数据，生成 favicon.ico 文件。
所以，上一版本使用
process.env?.NODE_ENV !== 'test'进行判断，跳过了 favicon.ico 的文件生成。重构版本之后，由于缺少
process环境变量，本地开发环境会导致该判断语句报错，导致工程模板生成失败。【问题修复方案】
process.env?.NODE_ENV !== 'test'的判断语句vi.mock模拟文件模块的导入，并判断返回结果，将结果写入到文件中。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
Bug Fixes
Tests