test(utils): [grid,select] add vitest for utils package #2813
Conversation
WalkthroughThis pull request involves a comprehensive refactoring of the utility modules within the Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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 (
|
Walkthrough此PR为@opentiny/utils包增加了vitest测试用例，简化了log的实现，删除了nanoid代码，简化了index的导出写法，并为grid添加了utils依赖。 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message) // 编码为（utf-8）Uint8Array | ||
| const hashBuffer = await window.crypto.subtle.digest(algo, msgUint8) // 计算消息的哈希值 | ||
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array | ||
| const hashBuffer = await getWindow().crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 |
There was a problem hiding this comment.
The getWindow().crypto.subtle.digest method is used here, which relies on the browser's Web Crypto API. Ensure that this method is supported in the environments where this code will run, as it may not be available in all JavaScript environments.
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
WalkthroughThis PR adds vitest test cases to the @opentiny/utils package, simplifies the implementation of log, deletes nanoid code, simplifies the way to export index, and adds utils dependency for grid. Changes
|
zzcr
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: 1
🧹 Nitpick comments (8)
packages/utils/src/crypt/__test__/crypt.test.ts (1)
4-7: Add a test case forArrayBufferinputThe current test covers the
sha256function with a string input. Sincesha256also accepts anArrayBuffer, consider adding a test case to verify that it handlesArrayBufferinputs correctly.Apply this diff to add the test case:
test('测试sha256', async () => { // 简单记录加密的结果，测试用来保证sha256算法不变化 expect(await sha256('hello world')).toMatchSnapshot() + const arrayBuffer = new TextEncoder().encode('hello world').buffer + expect(await sha256(arrayBuffer)).toMatchSnapshot() })packages/utils/src/log/index.ts (2)
3-3: Consider using a more specific type thanany.Using
anytype reduces type safety. Consider using a more specific type that includes the console property.-const _win: any = getWindow() +interface WindowWithConsole { + console: Console; +} +const _win = getWindow() as WindowWithConsole
4-5: Add JSDoc documentation for the exported log utility.The exported
logutility would benefit from comprehensive JSDoc documentation in English, including:
- Purpose of the utility
- Usage examples
- Type information
+/** + * A wrapper around console to provide consistent logging across the application + * @example + * import { log } from '@opentiny/utils' + * log.logger.warn('Warning message') + */ export const log = { logger: _win.console as Console }packages/vue/src/grid/src/tools/logger.ts (1)
Line range hint
4-12: Add null check for i18n message.The i18n message lookup could return undefined. Consider adding a fallback for undefined translations.
const outLog = (type) => (message, detail) => { - let msg = `[tiny-grid] ${GlobalConfig.i18n(message) || message}` + const translatedMessage = GlobalConfig.i18n(message) + let msg = `[tiny-grid] ${translatedMessage !== undefined ? translatedMessage : message}` if (detail) { msg += `: ${detail}` } log.logger.log(msg, type) return msg }packages/utils/src/xss/__test__/xss.test.ts (1)
4-17: Enhance test coverage with additional test cases.While the current tests cover basic scenarios, consider adding:
- Edge cases (empty strings, null values)
- Complex HTML with nested elements
- URLs with query parameters and fragments
- International characters in URLs
Example additional test cases:
// Edge cases expect(filterHtml('')).toMatchInlineSnapshot(`""`) expect(filterUrl('')).toMatchInlineSnapshot(`""`) // Complex HTML expect(filterHtml(`<div><script>alert('XSS')</script><p>Safe</p></div>`)) .toMatchInlineSnapshot(`"<div><p>Safe</p></div>"`) // URLs with query params expect(filterUrl(`https://s.com/user?id=1&name=test#hash`)) .toMatchInlineSnapshot(`"https://s.com/user?id=1&name=test#hash"`) // International URLs expect(filterUrl(`https://测试.com/path`)) .toMatchInlineSnapshot(`"https://测试.com/path"`)packages/renderless/src/common/index.ts (1)
13-13: Consider using TypeScript enums for constants.The file contains several constant objects that could be converted to TypeScript enums for better type safety and IDE support.
Example transformation:
export enum Position { Left = 'left', Right = 'right', Top = 'top', Bottom = 'bottom' } export enum Sort { Asc = 'asc', Desc = 'desc' }packages/utils/README.md (1)
1-5: Enhance documentation with testing instructions.Since this PR introduces Vitest testing capabilities, consider adding:
- A section about running tests (
npm test)- Examples of writing tests
- Test coverage information
Also, consider maintaining consistency in language usage (Chinese vs English).
packages/utils/package.json (1)
20-20: Enhance test script configuration.Consider adding more comprehensive test scripts:
- "test": "vitest" + "test": "vitest", + "test:watch": "vitest watch", + "test:coverage": "vitest run --coverage", + "test:ui": "vitest --ui"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/utils/src/crypt/__test__/__snapshots__/crypt.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (19)
packages/mobile/components/file-upload/src/renderless/index.ts(4 hunks)packages/renderless/src/common/index.ts(1 hunks)packages/renderless/src/tree/index.ts(1 hunks)packages/utils/README.md(1 hunks)packages/utils/package.json(2 hunks)packages/utils/src/crypt/__test__/crypt.test.ts(1 hunks)packages/utils/src/crypt/core.ts(0 hunks)packages/utils/src/crypt/index.ts(1 hunks)packages/utils/src/crypt/sha256.ts(0 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/log/index.ts(1 hunks)packages/utils/src/logger/README.md(0 hunks)packages/utils/src/logger/index.ts(0 hunks)packages/utils/src/nanoid/README.md(0 hunks)packages/utils/src/nanoid/index.ts(0 hunks)packages/utils/src/nanoid/nanoid.ts(0 hunks)packages/utils/src/xss/__test__/xss.test.ts(1 hunks)packages/vue/src/grid/package.json(1 hunks)packages/vue/src/grid/src/tools/logger.ts(2 hunks)
💤 Files with no reviewable changes (7)
- packages/utils/src/logger/README.md
- packages/utils/src/crypt/sha256.ts
- packages/utils/src/nanoid/README.md
- packages/utils/src/logger/index.ts
- packages/utils/src/crypt/core.ts
- packages/utils/src/nanoid/index.ts
- packages/utils/src/nanoid/nanoid.ts
✅ Files skipped from review due to trivial changes (2)
- packages/renderless/src/tree/index.ts
- packages/mobile/components/file-upload/src/renderless/index.ts
🔇 Additional comments (4)
packages/utils/src/index.ts (1)
1-10: Simplification of imports and exports looks goodThe refactoring of module imports and exports enhances code readability and removes unnecessary dependencies. The removal of
nanoidappears to be intentional and aligns with the PR objectives.packages/utils/package.json (2)
3-3: LGTM! ESM configuration added.The addition of
"type": "module"aligns with modern JavaScript practices.
4-6: Update package description.The description still mentions "nanoid" which has been removed according to the PR objectives. Consider updating it to better reflect the current package contents.
Let's verify if nanoid is actually removed:
✅ Verification successful
The package description is indeed outdated and should be updated
The verification confirms that
nanoidhas been removed from the codebase (as evidenced by the changelog) with no remaining dependencies or imports. The only remaining reference is in the package description itself, which should be updated to accurately reflect the current package contents.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if nanoid still exists in the codebase rg -g '!package*.json' 'nanoid' packages/utils/Length of output: 47
Script:
#!/bin/bash # Check package.json for nanoid dependency echo "=== Checking package.json for nanoid ===" cat packages/utils/package.json | grep -i nanoid # Search for import statements with nanoid echo -e "\n=== Checking for nanoid imports ===" rg "import.*nanoid|require.*nanoid" packages/utils/ # Search entire repo for nanoid references echo -e "\n=== Checking entire repo for nanoid ===" rg -g '!package-lock.json' -g '!yarn.lock' "nanoid"Length of output: 703
packages/vue/src/grid/package.json (1)
29-30: LGTM! Dependency addition looks good.The addition of
@opentiny/utilsas a workspace dependency is correctly configured.
| export async function sha256(message: ArrayBuffer | string) { | ||
| const isArrayBuffer = Object.prototype.toString.call(message) === '[object ArrayBuffer]' | ||
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message) // 编码为（utf-8）Uint8Array | ||
| const hashBuffer = await window.crypto.subtle.digest(algo, msgUint8) // 计算消息的哈希值 | ||
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array | ||
| const hashBuffer = await getWindow().crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 | ||
| const hashArray = Array.from(new Uint8Array(hashBuffer)) // 将缓冲区转换为字节数组 | ||
| const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('') // 将字节数组转换为十六进制字符串 | ||
|
|
||
| return hashHex | ||
| } | ||
|
|
There was a problem hiding this comment.
Consider adding error handling for unsupported environments
The sha256 function assumes that getWindow().crypto.subtle.digest is always available. In environments where the Web Crypto API is not supported, this will lead to runtime errors. To ensure broader compatibility, consider adding error handling or a fallback implementation.
Apply this diff to add error handling:
export async function sha256(message: ArrayBuffer | string) {
const isArrayBuffer = Object.prototype.toString.call(message) === '[object ArrayBuffer]'
const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array
+ const crypto = getWindow().crypto
+ if (!crypto || !crypto.subtle || typeof crypto.subtle.digest !== 'function') {
+ throw new Error('Web Crypto API is not supported in this environment.')
+ }
const hashBuffer = await crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值
const hashArray = Array.from(new Uint8Array(hashBuffer)) // 将缓冲区转换为字节数组
const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('') // 将字节数组转换为十六进制字符串
return hashHex
}📝 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.
| export async function sha256(message: ArrayBuffer | string) { | |
| const isArrayBuffer = Object.prototype.toString.call(message) === '[object ArrayBuffer]' | |
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message) // 编码为（utf-8）Uint8Array | |
| const hashBuffer = await window.crypto.subtle.digest(algo, msgUint8) // 计算消息的哈希值 | |
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array | |
| const hashBuffer = await getWindow().crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 | |
| const hashArray = Array.from(new Uint8Array(hashBuffer)) // 将缓冲区转换为字节数组 | |
| const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('') // 将字节数组转换为十六进制字符串 | |
| return hashHex | |
| } | |
| export async function sha256(message: ArrayBuffer | string) { | |
| const isArrayBuffer = Object.prototype.toString.call(message) === '[object ArrayBuffer]' | |
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array | |
| const crypto = getWindow().crypto | |
| if (!crypto || !crypto.subtle || typeof crypto.subtle.digest !== 'function') { | |
| throw new Error('Web Crypto API is not supported in this environment.') | |
| } | |
| const hashBuffer = await crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 | |
| const hashArray = Array.from(new Uint8Array(hashBuffer)) // 将缓冲区转换为字节数组 | |
| const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('') // 将字节数组转换为十六进制字符串 | |
| return hashHex | |
| } |
PR
为@opentiny/utils增加vitest的测试用例
简化 log的实现
删除nanoid的代码
简化总的index的导出写法
添加grid的utils依赖
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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
Refactor
Documentation
Tests
Chores