fix(utils): delete the renderless/common directory and adjust all reference paths. #2849
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 123 files out of 230 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis pull request involves a comprehensive refactoring of dependency management and utility functions across multiple files. The changes primarily focus on removing the Changes
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 (
|
WalkthroughThis PR refactors the utility functions by adjusting the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| const isArrayBuffer = Object.prototype.toString.call(message) === '[object ArrayBuffer]' | ||
| const msgUint8 = isArrayBuffer ? message : new TextEncoder().encode(message as string) // 编码为（utf-8）Uint8Array | ||
| const hashBuffer = await getWindow().crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 | ||
| const hashBuffer = globalThis.crypto.subtle.digest('SHA-256', msgUint8) // 计算消息的哈希值 |
There was a problem hiding this comment.
The removal of await from the sha256 function suggests that crypto.subtle.digest is now expected to be synchronous, which is incorrect. crypto.subtle.digest returns a Promise, so the function should remain asynchronous, and await should be used here.
|
[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 |
![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
🔭 Outside diff range comments (1)
packages/utils/src/index.ts (1)
Line range hint
11-200: Resolve naming conflicts in utility functions.Several utilities have numbered suffixes (e.g.,
isDate1,toDate1,getScrollParent1) which indicates potential naming conflicts. Consider:
- Consolidating duplicate functionality
- Using more descriptive names instead of numeric suffixes
- Documenting the differences between similar functions
Example for better naming:
- export { getScrollParent as getScrollParent1 } from './popper' + export { getScrollParent as getPopperScrollParent } from './popper'
🧹 Nitpick comments (3)
packages/utils/src/logger/index.ts (2)
1-1: Consider adding type safety for globalThis.While using
globalThisis a modern approach, the current implementation withanytype might be too permissive. Consider adding proper type definitions to ensure type safety.-const _win: any = globalThis +const _win: typeof globalThis = globalThis
2-2: Consider translating the Chinese comment to English.For better international collaboration, consider translating the Chinese comment "使用 logger.xxx 代替 window.console.xxx, 避免语法警告" to English.
-/** 使用 logger.xxx 代替 window.console.xxx, 避免语法警告 */ +/** Use logger.xxx instead of window.console.xxx to avoid syntax warnings */packages/utils/src/index.ts (1)
Line range hint
11-200: Technical debt should be tracked in issues.The file contains multiple TODO comments indicating pending refactoring tasks:
- Moving global variables to component-specific locations
- Consolidating date-related utilities
- Removing or relocating various utilities
These should be tracked as separate issues for better visibility and planning.
Would you like me to help create issues to track these technical debt items? I can help generate detailed issue descriptions for each TODO comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/nuxt/playground/package.json(1 hunks)examples/vue2/package.json(1 hunks)examples/vue3/package.json(1 hunks)packages/renderless/src/file-upload/index.ts(3 hunks)packages/utils/src/crypt/index.ts(1 hunks)packages/utils/src/globalConfig/index.ts(0 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/logger/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/utils/src/globalConfig/index.ts
🔇 Additional comments (7)
packages/utils/src/logger/index.ts (1)
Line range hint
1-5: Verify browser compatibility with the globalThis change.The switch from
getWindow()toglobalThismight affect browser compatibility. WhileglobalThisis well-supported in modern browsers, we should verify compatibility requirements.✅ Verification successful
The
globalThischange is compatible with the library's browser support requirementsThe switch from
getWindow()toglobalThisis safe because:
- The library is built on Vue 3 which already requires modern browsers
globalThishas broad support in all modern browsers since 2019- The change is isolated to the logger utility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any browser compatibility requirements specified fd -t f "package.json|browserslist|.browserslistrc" -x cat {} \; # Check if globalThis is used consistently across the codebase rg "globalThis|getWindow" --type tsLength of output: 65784
packages/utils/src/crypt/index.ts (1)
2-9: LGTM! Good improvements to the SHA256 implementation.The changes improve the code by:
- Using the standardized
globalThisinstead of a custom window detection- Simplifying the implementation by removing unnecessary async/await
- Maintaining the same functionality and output format
Let's verify the browser compatibility of
globalThis.crypto:✅ Verification successful
The change is safe for the project's browser targets
The codebase explicitly targets modern browsers (>1%, last 2 versions, no IE) where both
globalThisandcrypto.subtleare well-supported. This is consistent with the existing usage ofglobalThisacross multiple files in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check browser compatibility of globalThis.crypto # Note: This will help ensure the changes don't break in older browsers curl -s "https://raw.githubusercontent.com/mdn/browser-compat-data/main/api/crypto.json" | \ jq -r '.api.crypto.support | to_entries[] | select(.value.version_added != null) | "\(.key): \(.value.version_added)"'Length of output: 273
Script:
#!/bin/bash # Check compatibility for both globalThis and crypto.subtle.digest # Check globalThis compatibility echo "=== globalThis compatibility ===" curl -s "https://raw.githubusercontent.com/mdn/browser-compat-data/main/javascript/builtins/globalThis.json" | \ jq -r '.javascript.builtins.globalThis.support | to_entries[] | select(.value.version_added != null) | "\(.key): \(.value.version_added)"' echo -e "\n=== crypto.subtle.digest compatibility ===" curl -s "https://raw.githubusercontent.com/mdn/browser-compat-data/main/api/SubtleCrypto.json" | \ jq -r '.api.SubtleCrypto.digest.support | to_entries[] | select(.value.version_added != null) | "\(.key): \(.value.version_added)"'Length of output: 771
Script:
#!/bin/bash # Search for crypto-related implementations and compatibility handling echo "=== Checking for crypto-related files ===" fd -t f -e ts -e js crypto echo -e "\n=== Checking for globalThis usage ===" rg "globalThis" -A 2 -B 2 echo -e "\n=== Checking for crypto polyfills or fallbacks ===" rg -i "polyfill|fallback|compat" . -g "*crypto*" echo -e "\n=== Checking package.json for browser targets ===" fd package.json --exec cat {} | jq -r '.browserslist // .targets // .engines // empty'Length of output: 2012
packages/renderless/src/file-upload/index.ts (2)
516-516: LGTM! Correctly adapted to sync SHA256.The change properly adapts to the new synchronous SHA256 implementation.
2049-2049: LGTM! Correctly adapted to sync SHA256.The change properly adapts to the new synchronous SHA256 implementation.
examples/nuxt/playground/package.json (1)
36-36: LGTM! Correctly removed unused js-sha256 dependency.The removal of js-sha256 is appropriate as the implementation now uses the native Web Crypto API through globalThis.crypto.
examples/vue3/package.json (1)
61-61: LGTM! Correctly removed unused js-sha256 dependency.The removal of js-sha256 is appropriate as the implementation now uses the native Web Crypto API through globalThis.crypto.
examples/vue2/package.json (1)
Line range hint
1-62: Verify the removal of js-sha256 dependency.The removal of
js-sha256dependency aligns with the PR objectives. However, we should ensure that any code using this dependency has been properly migrated.✅ Verification successful
Removal of js-sha256 is safe and represents an improvement
The dependency has been properly replaced with Web Crypto API's native SHA256 implementation in
packages/utils/src/crypt/index.ts. The new implementation is more secure, potentially faster, and reduces external dependencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of js-sha256 in the codebase # Search for any remaining imports or usage of js-sha256 echo "Checking for js-sha256 imports or usage..." rg -l "js-sha256|require\(['\"']js-sha256['\"]" || echo "No js-sha256 imports found" # Search for SHA256-related functionality that might need the library echo -e "\nChecking for SHA256-related code..." rg -l "sha256|SHA256" || echo "No SHA256-related code found"Length of output: 610
Script:
#!/bin/bash # Check the cryptography implementation and its tests echo "=== Checking cryptography implementation ===" cat packages/utils/src/crypt/index.ts echo -e "\n=== Checking cryptography tests ===" cat packages/utils/src/crypt/__test__/crypt.test.ts echo -e "\n=== Checking test snapshots ===" cat packages/utils/src/crypt/__test__/__snapshots__/crypt.test.ts.snapLength of output: 1350
packages/utils/src/index.ts
Outdated
| export { xss, logger, ResizeObserver } | ||
|
|
||
| export { sha256 } from './crypt' | ||
| export { globalConfig, getViewportWindow } from './globalConfig' |
There was a problem hiding this comment.
💡 Codebase verification
Breaking change detected in mobile utils
The export reorganization is properly implemented, but there's one breaking change that needs to be addressed:
- Update
packages/mobile/utils/xss.tsto usegetViewportWindow()instead ofgetWindow()in the line:let log = { logger: getWindow().console }
🔗 Analysis chain
Export reorganization looks good but needs documentation.
The reorganization of exports and separation of sha256 from the crypt module improves modularity. However, since this is a breaking change, it should be documented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential breaking changes impact
# Search for direct imports of the removed crypt module
echo "Checking for imports of the removed crypt module..."
rg "import.*from ['\"]@opentiny/vue-utils/crypt['\"]" || echo "No direct crypt imports found"
# Search for destructured imports of getWindow and isWeb
echo -e "\nChecking for imports of removed getWindow and isWeb utilities..."
rg "import.*\{.*(getWindow|isWeb).*\}.*from ['\"]@opentiny/vue-utils['\"]" || echo "No getWindow/isWeb imports found"
Length of output: 542
Script:
#!/bin/bash
# Check for TypeScript references and indirect usages
# Check for type references
echo "Checking for TypeScript type references..."
rg "import type.*\{.*(getWindow|isWeb).*\}.*from ['\"]@opentiny/vue-utils['\"]" || echo "No type references found"
# Check implementation of new exports
echo -e "\nVerifying new exports implementation..."
ast-grep --pattern 'export { sha256 } from "./crypt"'
ast-grep --pattern 'export { globalConfig, getViewportWindow } from "./globalConfig"'
# Check for any references to these functions in the codebase
echo -e "\nChecking for any function references..."
rg "getWindow\s*\(" || echo "No getWindow function calls found"
rg "isWeb\s*\(" || echo "No isWeb function calls found"
Length of output: 753
Script:
#!/bin/bash
# Verify the actual implementation files
echo "Checking crypt.ts content..."
fd -e ts crypt.ts --exec cat {}
echo -e "\nChecking globalConfig.ts content..."
fd -e ts globalConfig.ts --exec cat {}
echo -e "\nChecking if getViewportWindow is the replacement for getWindow..."
rg "getViewportWindow" -A 3
Length of output: 3041
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/renderless/src/common/string.ts (1)
Line range hint
592-601: Fix potential infinite loop in text truncation.The current implementation has a risk of infinite loop if the text cannot be truncated to fit within the specified width. Add a check to prevent this.
} else { + if (text.length <= 3) return { t: '...', o: true } for (let i = -1; ; i--) { t = text.slice(0, i) + '...' metric = ctx.measureText(t) if (metric.width < w) { return { t, o: true } } + if (t.length <= 3) return { t: '...', o: true } } }
🧹 Nitpick comments (15)
packages/renderless/src/common/string.ts (1)
Line range hint
584-586: Add proper TypeScript types for canvas context.The canvas context should be properly typed to ensure type safety.
- const canvas = document.createElement('canvas') - const ctx = canvas.getContext('2d') + const canvas = document.createElement('canvas') + const ctx = canvas.getContext('2d') as CanvasRenderingContext2Dpackages/utils/src/resize-observer/index.ts (1)
100-100: Consider fallback for legacy environments.Using
globalThis.Functionis modern and neat. However, older environments might lackglobalThis. If legacy support is crucial, consider adding a fallback or polyfill.packages/vue/src/virtual-tree/src/index.ts (2)
11-14: Default row height set to 36.This is a common default for row-based layouts. If your design requires further customization, consider exposing a prop or documenting the recommended usage.
19-19:treeOp: nullremoves type safety.Using
nullmeans no type checks or constraints fortreeOp. If you need to enforce or document any expected structure, consider defining a more explicit type.packages/vue/src/virtual-scroll-box/src/index.ts (1)
11-12: Row and column buffer defaults.Defaults of 200 for
rowBufferandcolumnBufferoffer a wide view area for pre-rendered rows/columns. Confirm this suits typical usage and memory constraints.packages/vue-common/src/breakpoint.ts (1)
2-2: LGTM! Consider separating environment checks.The change correctly imports
isServerfrom the centralized location.Consider separating the environment checks for better clarity:
- if (isServer || typeof matchMedia !== 'function') { + if (isServer) { + return { + matches: false, + media: mediaQueryString, + addEventListener: () => {}, + removeEventListener: () => {} + } + } + + if (typeof matchMedia !== 'function') { return { matches: false, media: mediaQueryString, addEventListener: () => {}, removeEventListener: () => {} } } else { return window.matchMedia(mediaQueryString) }packages/utils/src/fastdom/singleton.ts (1)
11-11: LGTM! Consider caching the RAF function.The change from
isBrowserto!isServermaintains the same logic. However, since RAF is called frequently for DOM operations, consider caching the result of the condition check.const RAF = (function () { + const isNotServer = !isServer - if (!isServer) { + if (isNotServer) { return window.requestAnimationFrame.bind(window) } return function (callback) {Also applies to: 14-14
packages/vue/src/float-button/src/index.ts (1)
14-14: LGTM! Consider improving type safety.The change from
isBrowserto!isServermaintains the same logic. However, the type could be more specific.element: { default: !isServer ? document.body : null, - type: !isServer ? HTMLElement : Object + type: !isServer ? HTMLElement : null }Also applies to: 98-99
packages/utils/src/touch-emulator/index.ts (1)
8-8: Consider caching the matches property.The
matchesproperty is assigned conditionally but could be accessed frequently. Consider caching the entireElement.prototype.matchescheck result.-const matches = !isServer ? Element.prototype.matches : null +const matches = !isServer && Element.prototype.matches || nullpackages/vue-hooks/src/vue-popup.ts (1)
15-17: Address TODO comment for type definitions.The file contains a TODO comment about types. Consider completing the type definitions for better type safety.
Would you like me to help implement the missing type definitions or create an issue to track this task?
packages/renderless/src/split/index.ts (1)
11-11: LGTM! The environment check refactoring improves consistency.The change from
isBrowsertoisServeraligns with the codebase-wide standardization of environment checks. The additional state check incomputeOffsetprovides better protection against undefined errors during component cleanup.Consider extracting the environment and state check into a separate utility function for reuse:
+const isValidEnvironment = (state) => state && !isServer + export const computeOffset = ({ api, nextTick, props, vm, state }) => () => { setTimeout(() => { - if (state && !isServer) { + if (isValidEnvironment(state)) { state.totalPane = vm.$refs.outerWrapper[state.offsetSize] state.leftTopPane = state.totalPane * (state.offset / 100) } })Also applies to: 175-175
packages/utils/src/espace-ctrl/index.ts (1)
Line range hint
384-396: Consider usingglobalThisfor WebSocket check.While the environment check is correct, the WebSocket check could be more robust by using
globalThis.-if (isServer || !window.WebSocket) { +if (isServer || !globalThis.WebSocket) { const notFn = function () { return undefined }packages/utils/src/globalConfig/index.ts (1)
24-33: Optimize Edge browser detection performance.The current implementation uses the
deleteoperator which can impact performance.Apply this diff to improve performance:
const isEdge = (browser) => { if (browser.chrome && ~navigator.userAgent.indexOf('Edg')) { browser.name = 'edge' browser.edge = true - delete browser.chrome + browser.chrome = undefined } else if (!document.documentMode && !!window.StyleMedia) { browser.name = 'edge' browser.edge = true } }🧰 Tools
🪛 Biome (1.9.4)
[error] 29-29: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/renderless/src/base-select/index.ts (1)
1868-1871: Consider using optional chaining for nextTick callback.The server check replacement looks good, but the code could be improved further:
- The
isBrowsertoisServerchange aligns with the codebase's server-side check pattern- The nextTick callback could benefit from optional chaining
Apply these improvements:
- if (isServer) return - nextTick(() => { - const parentEl = parent.$el - const inputEl = parentEl.querySelector('input[data-tag="tiny-input-inner"]') + if (isServer) return + nextTick(() => { + const inputEl = parent.$el?.querySelector('input[data-tag="tiny-input-inner"]')packages/vue/src/grid/src/table/src/table.ts (1)
1014-1014: LGTM! Consider adding a comment for clarity.The change from
isBrowserto!isServeris a good modernization that aligns with best practices for environment detection. However, since this is a critical environment check, consider adding a brief comment explaining the logic.- relationContainer: () => (!isServer ? $table.$el.querySelector(`.${hiddenContainerClass}`) : null), + relationContainer: () => (!isServer ? $table.$el.querySelector(`.${hiddenContainerClass}`) : null), // Skip DOM operations on server-side
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
packages/renderless/src/base-select/index.ts(2 hunks)packages/renderless/src/cascader-select/column-index.ts(2 hunks)packages/renderless/src/cascader/index.ts(2 hunks)packages/renderless/src/common/browser.ts(0 hunks)packages/renderless/src/common/deps/clickoutside.ts(1 hunks)packages/renderless/src/common/string.ts(2 hunks)packages/renderless/src/image-viewer/index.ts(1 hunks)packages/renderless/src/input/index.ts(2 hunks)packages/renderless/src/milestone/index.ts(1 hunks)packages/renderless/src/nav-menu/index.ts(2 hunks)packages/renderless/src/picker/index.ts(0 hunks)packages/renderless/src/select-dropdown/vue.ts(2 hunks)packages/renderless/src/select/index.ts(2 hunks)packages/renderless/src/select/vue.ts(2 hunks)packages/renderless/src/split/index.ts(3 hunks)packages/renderless/src/tabs-mf/wheel.ts(3 hunks)packages/renderless/src/time-spinner/index.ts(2 hunks)packages/renderless/src/upload-list/vue.ts(2 hunks)packages/utils/src/bigInt/index.ts(0 hunks)packages/utils/src/browser/index.ts(0 hunks)packages/utils/src/dom/index.ts(1 hunks)packages/utils/src/espace-ctrl/index.ts(3 hunks)packages/utils/src/fastdom/singleton.ts(1 hunks)packages/utils/src/fullscreen/screenfull.ts(5 hunks)packages/utils/src/globalConfig/index.ts(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/popper/index.ts(3 hunks)packages/utils/src/popup-manager/index.ts(1 hunks)packages/utils/src/prop-util/index.ts(0 hunks)packages/utils/src/resize-event/index.ts(1 hunks)packages/utils/src/resize-observer/index.ts(5 hunks)packages/utils/src/scroll-into-view/index.ts(1 hunks)packages/utils/src/scroll-width/index.ts(1 hunks)packages/utils/src/string/index.ts(3 hunks)packages/utils/src/touch-emulator/index.ts(2 hunks)packages/vue-common/src/breakpoint.ts(1 hunks)packages/vue-directive/src/clickoutside.ts(1 hunks)packages/vue-hooks/src/vue-popper.ts(1 hunks)packages/vue-hooks/src/vue-popup.ts(1 hunks)packages/vue/src/float-button/src/index.ts(2 hunks)packages/vue/src/grid/src/table/src/table.ts(2 hunks)packages/vue/src/image/src/index.ts(2 hunks)packages/vue/src/virtual-scroll-box/src/index.ts(1 hunks)packages/vue/src/virtual-tree/src/index.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/renderless/src/common/browser.ts
- packages/renderless/src/picker/index.ts
- packages/utils/src/bigInt/index.ts
- packages/utils/src/browser/index.ts
- packages/utils/src/prop-util/index.ts
🧰 Additional context used
🪛 eslint
packages/renderless/src/split/index.ts
[error] 45-45: Insert ··
(prettier/prettier)
[error] 46-46: Insert ··
(prettier/prettier)
🪛 Biome (1.9.4)
packages/renderless/src/base-select/index.ts
[error] 1866-1868: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/utils/src/globalConfig/index.ts
[error] 29-29: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (45)
packages/renderless/src/common/string.ts (1)
246-246: LGTM! Secure random number generation usingglobalThis.The implementation correctly uses
globalThis.crypto.getRandomValuesfor cryptographically secure random number generation.packages/utils/src/resize-observer/index.ts (4)
15-15: Confirm SSR environment detection implementation.Switching to
isServeris consistent with a server-centric approach. However, please verify thatisServeris correctly configured for all SSR contexts to avoid mismatches in browser-based or hybrid scenarios.
219-219: Skip connect logic on the server side.Preventing DOM-based event logic when
isServerorthis.connected_is true makes sense and protects against SSR-related errors.
246-246: No-op disconnect on server side seems harmless.If we are already disconnected or running in SSR mode, safely skipping teardown code helps avoid unnecessary operations.
400-400: Return an empty rectangle in SSR mode.Returning
emptyRectaligns well with expectations in a server environment and prevents layout computations that depend on the DOM.packages/vue/src/virtual-tree/src/index.ts (2)
9-10: Flexible prop types for width and height.Defining them as
[Number, String]allows consumers to set numeric or string-based dimensions (e.g., percentages). Ensure the component logic gracefully handles string values.
15-18: Scrollbar size defaulted to 6.While 6px is a minimal scrollbar dimension, it may be too small for certain accessibility requirements. Confirm it meets your project's UX guidelines.
packages/vue/src/virtual-scroll-box/src/index.ts (3)
9-10: Flexible width and height definitions.Allowing
[Number, String]grants better control (e.g.,100%,500px). Be sure to handle string values correctly in calculations.
13-13: Scrollbar size declared as[Number, String].This allows for dynamic or fluid scrollbar sizing. Verify you're applying appropriate cast or logic when working with these values.
14-20: Arrays with arrow function defaults.Defining arrays as
default: () => []is a recommended Vue pattern, ensuring each component gets its own array instance. Good job. Also, verify that empty arrays won't cause runtime issues if the rest of the component expects populated data.packages/utils/src/scroll-into-view/index.ts (1)
13-13: LGTM! Centralizing server-side detection.The change correctly imports
isServerfrom the centralized location, maintaining the same functionality while improving maintainability.Let's verify that the imported
isServeris properly exported from globalConfig:✅ Verification successful
Import verification successful
The
isServerconstant is properly exported fromglobalConfig/index.tswith the correct server-side detection implementation. The import inscroll-into-view/index.tswill work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify isServer export in globalConfig rg -l "export.*isServer" "packages/utils/src/globalConfig"Length of output: 99
Script:
#!/bin/bash # View the isServer export implementation rg "export.*isServer" -A 2 "packages/utils/src/globalConfig/index.ts"Length of output: 153
packages/utils/src/resize-event/index.ts (1)
14-14: LGTM! Proper SSR handling maintained.The change correctly imports
isServerwhile maintaining proper server-side rendering compatibility for the ResizeObserver functionality.packages/utils/src/scroll-width/index.ts (1)
13-14: LGTM! Proper DOM handling maintained.The change correctly imports
isServerwhile maintaining proper DOM manipulation guards for server-side rendering.packages/vue/src/image/src/index.ts (1)
15-15: LGTM! Consistent environment check update.The change from
isBrowserto!isServermaintains the same logic while aligning with the new environment detection approach.Also applies to: 39-39
packages/renderless/src/tabs-mf/wheel.ts (2)
1-1: LGTM! Environment check in detectEventModel is correct.The change from
isBrowserto!isServermaintains the same logic for detecting the event model.Also applies to: 25-25
Line range hint
35-44: LGTM! Early return pattern improves readability.The early return for server environment is a good pattern. The wheel event detection is correctly isolated to browser environments.
packages/utils/src/touch-emulator/index.ts (1)
Line range hint
117-122: Verify touch emulation in SSR environments.The touch emulation logic has been updated to use
isServercheck. Please ensure this doesn't affect server-side rendering or hydration.✅ Verification successful
Touch emulation is properly guarded for SSR environments
The implementation correctly handles server-side rendering by:
- Guarding all browser-specific code with
isServerchecks- Consistent environment detection across touch-related features
- Proper protection of window/document access
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for touch event related code to verify consistent environment checks rg -l "touchstart|touchmove|touchend" | xargs rg "isServer|typeof window"Length of output: 1247
Error: No shell scripts found
Script:
#!/bin/bash # Find and show the globalConfig implementation fd globalConfig.ts cat packages/utils/src/globalConfig.tsLength of output: 128
packages/vue-directive/src/clickoutside.ts (1)
13-13: LGTM! Improved server-side handling.The change to use imported
isServerimproves consistency and the event listeners are correctly conditional.Also applies to: 20-33
packages/renderless/src/common/deps/clickoutside.ts (1)
15-16: Consider consolidating duplicate clickoutside implementations.This file appears to duplicate functionality from
packages/vue-directive/src/clickoutside.ts. Consider consolidating these implementations to reduce maintenance overhead.packages/vue-hooks/src/vue-popup.ts (1)
Line range hint
108-112: Verify popup behavior in SSR applications.The
doOpenfunction usesisServercheck. Please ensure this doesn't affect popup behavior during server-side rendering and hydration.✅ Verification successful
Popup implementation is SSR-safe ✅
The popup behavior is properly handled during server-side rendering:
- The
doOpenfunction includes anisServercheck that prevents client-side operations during SSR- The PopupManager properly initializes its state and DOM-related properties as null for SSR
- Modal stack and DOM manipulations are only performed on the client side
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for popup-related code to verify consistent environment checks rg "isServer.*popup|popup.*isServer" -A 3Length of output: 43
Script:
#!/bin/bash # Search for server-side rendering related checks with various terms echo "=== Searching for server-side checks ===" rg "isServer|typeof window|typeof document" -A 3 echo -e "\n=== Looking for doOpen function ===" ast-grep --pattern 'function doOpen() { $$$ }' echo -e "\n=== Finding PopupManager usage ===" rg "PopupManager\." -A 3Length of output: 66090
Script:
#!/bin/bash # Find popup-related files and their content echo "=== Looking for popup files ===" fd "popup|vue-popup" -t f echo -e "\n=== Examining popup implementation ===" rg "doOpen" -A 5 packages/vue-hooks/src/vue-popup.ts echo -e "\n=== Looking for PopupManager implementation ===" rg "class PopupManager|const PopupManager" -A 10Length of output: 4682
packages/renderless/src/upload-list/vue.ts (1)
36-36: LGTM! Clean refactoring of environment detection.The changes correctly replace browser detection with server-side detection, maintaining the same logical flow while improving code consistency.
Also applies to: 69-69
packages/utils/src/fullscreen/screenfull.ts (2)
Line range hint
59-73: LGTM! Proper early return for server environment.The early return in
getFullScreenEventsensures that fullscreen events are not initialized in a server environment.
Line range hint
158-195: Well-structured fallbacks for server environment.Good addition of server-side fallbacks for fullscreen properties, ensuring consistent behavior across environments.
packages/renderless/src/milestone/index.ts (1)
22-22: LGTM! Safe CSS variable handling.The changes correctly handle CSS variables by checking for server environment before accessing DOM APIs.
Also applies to: 25-28
packages/utils/src/popup-manager/index.ts (1)
15-15: LGTM! Consistent environment check implementation.The import of
isServerfromglobalConfigaligns with the project's standardization of environment detection. The implementation maintains proper handling of server-side rendering scenarios.packages/vue-hooks/src/vue-popper.ts (1)
18-19: LGTM! Environment check standardization.The import of
isServerfrom@opentiny/utilsmaintains consistency with the project's standardized approach to environment detection.packages/utils/src/espace-ctrl/index.ts (1)
Line range hint
403-414: LGTM! Proper initialization guard.The initialization logic correctly prevents server-side execution while maintaining the same functionality.
packages/utils/src/dom/index.ts (1)
15-15: LGTM! Centralizing server detection.The change to import
isServerfrom@opentiny/utilsimproves maintainability by centralizing server detection logic.packages/renderless/src/time-spinner/index.ts (1)
16-16: LGTM! Standardizing server detection.The change to import
isServerfrom@opentiny/utilsaligns with the project-wide standardization of server detection.packages/renderless/src/input/index.ts (2)
14-14: LGTM! Standardizing server detection.The change to import
isServerfrom@opentiny/utilsaligns with the project-wide standardization of server detection.
458-461: Great documentation improvement!The added comments clearly explain the text width calculation logic and potential whitespace issues, which helps prevent misunderstandings and debugging challenges.
packages/renderless/src/nav-menu/index.ts (1)
27-27: LGTM! Standardizing server detection.The change to import and use
isServerfrom@opentiny/utilsmaintains the same logical behavior while standardizing server detection across the codebase.Also applies to: 494-494
packages/renderless/src/cascader/index.ts (2)
13-13: LGTM! Import statement updated for better environment detection.The change to import
isServerfrom@opentiny/utilsaligns with the codebase's transition to centralized environment detection.
359-359: LGTM! Condition updated for better server-side rendering support.The change from
if (isBrowser)toif (!isServer)maintains the same logic while making the server-side rendering check more explicit.packages/renderless/src/select/vue.ts (2)
118-118: LGTM! Import statement updated for better environment detection.The change to import
isServerfrom@opentiny/utilsaligns with the codebase's transition to centralized environment detection.
209-209: LGTM! Computed property updated for better server-side rendering support.The change to use
!isServermaintains the same logic while making the server-side rendering check more explicit.packages/utils/src/string/index.ts (2)
246-246: LGTM! Updated to use standardglobalThis.The change to use
globalThisinstead ofglobalEnvironmentimproves cross-environment compatibility by using the standard global object reference.
814-814: LGTM! Condition updated for better server-side rendering support.The change to use
isServeraligns with the codebase's transition to server-centric checks.packages/renderless/src/image-viewer/index.ts (1)
18-18: LGTM! Simplified event handling.The change to hardcode
mousewheelEventNameto'mousewheel'standardizes the event name across all browsers, removing unnecessary browser-specific handling.packages/utils/src/globalConfig/index.ts (1)
8-8: LGTM! Clean isServer implementation.The
isServerconstant provides a simple and effective way to detect server-side rendering environments.packages/utils/src/index.ts (1)
5-8: LGTM! Clean export organization.The reorganization of exports improves modularity by:
- Separating the
sha256function from thecryptmodule- Grouping related exports from
globalConfigpackages/renderless/src/select-dropdown/vue.ts (1)
99-99: LGTM! Consistent environment check.The change to use
!isServerin the watch condition aligns with the standardized approach for environment detection across the codebase.packages/utils/src/popper/index.ts (1)
277-277: LGTM: Browser check replaced with server check.The change from
isBrowserto!isServermaintains the same logic while aligning with the codebase's move towards server-side checks.Run this script to verify all browser checks have been consistently updated:
✅ Verification successful
Browser check refactoring is correctly isolated to utils package
The change from
isBrowserto!isServeris correctly implemented and doesn't require changes in other packages as they maintain their own browser detection utilities.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining isBrowser imports or usages rg "isBrowser" --type tsLength of output: 3444
Script:
#!/bin/bash # Find all popper-related files echo "=== Finding popper files ===" fd popper # Check content of the specific file echo -e "\n=== Checking file content ===" cat "packages/utils/src/popper/index.ts" 2>/dev/null || echo "File not found"Length of output: 30487
packages/renderless/src/select/index.ts (2)
2401-2404: Good addition of SSR compatibility check!The early return when
isServeris true prevents DOM manipulation code from running on the server side, which is essential for proper Server-Side Rendering (SSR) support.
19-19: LGTM: Import statement is correctly placed.The import of
isServerfrom '@opentiny/utils' is properly added and aligns with its usage in thewatchShowClosefunction.
| @@ -1,4 +1,4 @@ | |||
| import { isBrowser } from '../common/browser' | |||
| import { isServer } from '@opentiny/utils' | |||
There was a problem hiding this comment.
Fix the server check condition in onTouchStart.
The current condition if (isServer) return will prevent touch handling in server environments, but allow it in server environments. This appears to be the opposite of the intended behavior, as touch events should only be handled in browser environments.
Apply this fix:
- if (isServer) return
+ if (!isServer) {
touch.start(event)
if (state.moving) {
const dom = vm.$refs.roller
// ... rest of the function
}
+ }Also applies to: 45-45
| // 整体的浏览器判断标准已过时 | ||
| if (!!window.chrome && (!!window.chrome.webstore || /^Google\b/.test(window.navigator.vendor))) { | ||
| browser.name = 'chrome' | ||
| browser.chrome = true | ||
| matches = navigator.userAgent.match(/chrome\/(\d+)/i) | ||
| browser.version = !!matches && !!matches[1] && parseInt(matches[1], 10) | ||
| matches = undefined | ||
| } else if (!!document.all || !!document.documentMode) { | ||
| browser.name = 'ie' | ||
| browser.version = getIEVersion() | ||
| browser.ie = true | ||
| } else if (typeof window.InstallTrigger !== 'undefined') { | ||
| // 过时的判断条件， 最新的ff中已经没有该属性。 | ||
| browser.name = 'firefox' | ||
| browser.firefox = true | ||
| } else if (Object.prototype.toString.call(window.HTMLElement).indexOf('Constructor') > 0) { | ||
| browser.name = 'safari' | ||
| browser.safari = true | ||
| } else if ((!!window.opr && !!window.opr.addons) || !!window.opera) { | ||
| browser.name = 'opera' | ||
| browser.opera = true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update browser detection logic to use modern APIs.
The current browser detection logic uses outdated methods:
window.chrome.webstoreis deprecateddocument.allis a legacy featureInstallTriggeris no longer available in modern FirefoxHTMLElement.toString()behavior is unreliable for Safari detection
Consider using more modern and reliable methods:
- User-Agent Client Hints API for Chrome/Edge
navigator.userAgentDatafor modern browsers- Feature detection instead of browser detection where possible
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/utils/src/index.ts (1)
7-8:⚠️ Potential issueDocument breaking changes in exports.
The removal of
getWindowandisWebin favor ofgetViewportWindowandisServerrepresents breaking changes that need to be documented. Please:
- Add migration guide for users of removed utilities
- Update relevant documentation
- Consider bumping the major version
🧹 Nitpick comments (9)
packages/vue/src/image-viewer/src/mobileTouch.ts (2)
69-89: Consider improving gesture detection logic.The swipe detection logic has several areas for improvement:
- Magic numbers for thresholds should be constants
- Inconsistent thresholds between X (10) and Y (100) directions
- Missing TypeScript types for event parameters
Consider applying these improvements:
+ const SWIPE_THRESHOLD_X = 10; + const SWIPE_THRESHOLD_Y = 10; - if (Math.abs(disX) > 10 || Math.abs(disY) > 100) { + if (Math.abs(disX) > SWIPE_THRESHOLD_X || Math.abs(disY) > SWIPE_THRESHOLD_Y) {Also, consider adding TypeScript types:
interface TouchPosition { x: number; y: number; } class TinyTouch { private tinyVueTouches: TouchPosition; start(e: TouchEvent): void { // ... } end(e: TouchEvent): void { // ... } move(e: TouchEvent): void { // ... } }
52-56: Consider adding a configurable long-press duration.The long-press detection uses a hardcoded 1000ms timeout. Consider making this configurable through the binding value.
+ interface TinyTouchOptions { + fn: Function; + longTapDelay?: number; + } constructor(element, tinyBinding, type) { // ... - that.tinyVueCallBack = isObject(tinyBinding.value) ? tinyBinding.value.fn : tinyBinding.value + const options = isObject(tinyBinding.value) ? tinyBinding.value : { fn: tinyBinding.value }; + that.tinyVueCallBack = options.fn; + that.longTapDelay = options.longTapDelay || 1000; // ... } start(e) { // ... - }, 1000) + }, this.longTapDelay)packages/vue/src/grid-toolbar/src/index.ts (1)
Line range hint
1012-1014: Consider adding TypeScript type annotations.While the functionality is correct, adding proper TypeScript type annotations would improve code maintainability and catch potential type-related issues at compile time. For example:
-applySettings({ columns, pageSize, updatedSorting }) { +interface SettingsConfig { + columns?: Array<{[key: string]: any}>; + pageSize?: number; + updatedSorting?: boolean; +} + +applySettings({ columns, pageSize, updatedSorting }: SettingsConfig) {packages/renderless/src/date-range/vue.ts (1)
Line range hint
1-1: Overall Assessment: Well-structured utility function migration.The changes consistently migrate date utility functions from
@opentiny/vue-renderless/common/deps/date-utilto@opentiny/utilswith appropriate renaming. The validation logic remains intact across all components.Consider adding migration notes to the PR description to help other developers understand the changes and update their code accordingly.
packages/utils/src/date-util/fecha.ts (1)
1-343: Well-structured date utility implementation.The implementation provides a comprehensive set of date manipulation utilities with proper:
- Internationalization support
- Format masks for common date formats
- Robust parsing and formatting logic
- Timezone handling
- Error handling for invalid dates
Consider adding JSDoc comments to document the public API methods and their parameters.
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: The regular expression includes this negated empty character class.
Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.(lint/correctness/noEmptyCharacterClassInRegex)
[error] 235-235: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
packages/renderless/src/date-range/index.ts (2)
198-198: Use optional chaining for better readability.The conditional check can be simplified using optional chaining.
Apply this diff to improve the code:
- state.rightDate = value && value[1] && state.unlinkPanels ? right : nextMonth1(state.leftDate) + state.rightDate = value?.[1] && state.unlinkPanels ? right : nextMonth1(state.leftDate)🧰 Tools
🪛 Biome (1.9.4)
[error] 198-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
529-529: Avoid assignment in expressions.Using assignments in expressions can make the code harder to understand. Consider using a separate assignment statement.
Apply this diff to improve readability:
- () => - (state.leftDate = nextMonth1(state.leftDate)) + () => { + state.leftDate = nextMonth1(state.leftDate) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 529-531: 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)
packages/renderless/src/date-panel/index.ts (1)
195-195: Avoid assignment in expressions.Using assignments in expressions can make the code harder to understand. Consider using a separate assignment statement.
Apply this diff to improve readability:
- () => - (state.date = nextMonth1(state.date)) + () => { + state.date = nextMonth1(state.date) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 195-195: 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)
packages/utils/src/index.ts (1)
Line range hint
171-171: Plan architectural improvements for component-specific utilities.Multiple TODO comments indicate needed architectural improvements:
- Component-specific utilities should be moved to their respective components
- Some utilities need conversion to hooks
- Better naming needed for utilities like FullscreenApi
Consider:
- Creating a tracking issue for these architectural improvements
- Prioritizing the conversion of touch utilities to hooks
- Moving component-specific utilities to their respective components to improve maintainability
Also applies to: 177-177, 196-196
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/renderless/src/calendar-bar/index.ts(1 hunks)packages/renderless/src/calendar-view/index.ts(1 hunks)packages/renderless/src/calendar/index.ts(1 hunks)packages/renderless/src/date-panel/index.ts(7 hunks)packages/renderless/src/date-panel/vue.ts(2 hunks)packages/renderless/src/date-range/index.ts(18 hunks)packages/renderless/src/date-range/vue.ts(2 hunks)packages/utils/src/date-util/fecha.ts(1 hunks)packages/utils/src/date-util/index.ts(1 hunks)packages/utils/src/index.ts(1 hunks)packages/vue/src/date-table/src/index.ts(2 hunks)packages/vue/src/grid-toolbar/src/index.ts(1 hunks)packages/vue/src/grid/src/body/src/body.tsx(2 hunks)packages/vue/src/grid/src/footer/src/footer.ts(1 hunks)packages/vue/src/grid/src/header/src/header.ts(1 hunks)packages/vue/src/grid/src/table/src/methods.ts(2 hunks)packages/vue/src/grid/src/table/src/table.ts(4 hunks)packages/vue/src/grid/src/tools/formatter.ts(1 hunks)packages/vue/src/image-viewer/src/mobileTouch.ts(4 hunks)packages/vue/src/month-table/package.json(1 hunks)packages/vue/src/month-table/src/pc.vue(2 hunks)packages/vue/src/recycle-scroller/src/idState.ts(1 hunks)packages/vue/src/year-table/src/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (10)
- packages/utils/src/date-util/index.ts
- packages/vue/src/grid/src/tools/formatter.ts
- packages/vue/src/grid/src/footer/src/footer.ts
- packages/vue/src/grid/src/header/src/header.ts
- packages/renderless/src/calendar/index.ts
- packages/renderless/src/calendar-bar/index.ts
- packages/vue/src/recycle-scroller/src/idState.ts
- packages/vue/src/grid/src/table/src/methods.ts
- packages/renderless/src/calendar-view/index.ts
- packages/vue/src/grid/src/body/src/body.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/grid/src/table/src/table.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/renderless/src/date-panel/index.ts
[error] 195-195: 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)
packages/renderless/src/date-range/index.ts
[error] 198-198: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 529-531: 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)
packages/utils/src/date-util/fecha.ts
[error] 26-26: The regular expression includes this negated empty character class.
Negated empty character classes match anything.
If you want to match against [, escape it [.
Otherwise, remove the character class or fill it.
(lint/correctness/noEmptyCharacterClassInRegex)
[error] 235-235: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (16)
packages/vue/src/image-viewer/src/mobileTouch.ts (2)
1-1: LGTM! Import changes align with the refactoring objectives.The changes to import
isObjectfrom@opentiny/utilsand the ESLint configuration are appropriate.Also applies to: 14-14
53-53: LGTM! Improved type safety with strict equality operators.The changes from
==to===enhance type safety and follow TypeScript best practices.Also applies to: 70-70, 74-74, 78-78, 82-82, 86-86, 91-91
packages/vue/src/month-table/package.json (3)
20-23: LGTM! Test dependencies added.The addition of test utilities (
@opentiny-internal/vue-test-utilsandvitest) suggests improved test coverage capabilities for this component.
14-18: Verify the usage of @opentiny/utils package.The addition of
@opentiny/utilsaligns with the PR's objective to adjust utility functions. Since this PR mentions removinggetWindowandisWeb, we should verify that any such usage in this component has been migrated to the new utilities.Let's check for any remaining usage of the old utilities:
✅ Verification successful
Dependency usage verified successfully
The
@opentiny/utilspackage is correctly used in the component, importing theisDate1utility. No instances of deprecated utilities (getWindow,isWeb) were found, and the dependency is properly utilized.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of deprecated utilities in the month-table component # Search for getWindow usage rg "getWindow|isWeb" "packages/vue/src/month-table" # Check imports from the new utils package rg "from ['\"]\@opentiny/utils['\"]" "packages/vue/src/month-table" -A 5Length of output: 233
Script:
#!/bin/bash # Search for any usage of getWindow or isWeb utilities rg -t js -t vue "getWindow|isWeb" packages/vue/src/month-table/ # Check imports from @opentiny/utils with fixed regex rg -t js -t vue "from ['\"]@opentiny/utils['\"]" packages/vue/src/month-table/ # Also check the renderless implementation rg -t js -t vue "getWindow|isWeb" packages/vue/src/month-table-renderless/Length of output: 311
Script:
#!/bin/bash # Find all files in the month-table directories first echo "Files in month-table directories:" find packages/vue/src/month-table* -type f echo -e "\nChecking for getWindow or isWeb usage:" rg "getWindow|isWeb" packages/vue/src/month-table* echo -e "\nChecking imports from @opentiny/utils:" rg "from ['\"]@opentiny/utils['\"]" packages/vue/src/month-table*Length of output: 631
3-3: Verify the impact of module type and side effects configuration.The addition of
"type": "module"and"sideEffects": falsecould affect:
- How the package is imported in consuming applications
- Tree-shaking optimization in build processes
Let's verify the module resolution and imports:
Also applies to: 6-7
✅ Verification successful
The module type and side effects configuration changes are safe ✅
The changes align with the codebase patterns and won't cause compatibility issues:
- ESM usage is consistent with the existing codebase
- No side effects were detected that would conflict with tree-shaking optimizations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential issues with ES modules configuration # Check if any CommonJS require() calls exist which might break rg "require\(['\"]@opentiny/vue-month-table['\"]" -A 5 # Check if any files still use .js extension with ESM syntax fd -e js -x grep -l "import.*from" {} \;Length of output: 19858
Script:
#!/bin/bash # Check for potential side effects in month-table component # Look for global state modifications ast-grep --pattern 'window.$_ = $_' packages/vue/src/month-table ast-grep --pattern 'document.$_ = $_' packages/vue/src/month-table # Check for direct DOM manipulations ast-grep --pattern 'document.createElement($_)' packages/vue/src/month-table ast-grep --pattern 'document.querySelector($_)' packages/vue/src/month-table # Look for lifecycle hooks that might have side effects ast-grep --pattern 'mounted() { $$$ }' packages/vue/src/month-table # Check for any global event listeners rg "addEventListener|removeEventListener" packages/vue/src/month-tableLength of output: 438
packages/vue/src/grid-toolbar/src/index.ts (2)
Line range hint
1012-1014: LGTM! Proper type checking before applying pagination settings.The
isNumberutility is correctly used to validate thepageSizeparameter before applying changes to the grid's pagination configuration.
25-25: LGTM! Moving utility function to a centralized location.The change to import
isNumberfrom@opentiny/utilsaligns with good architectural practices by centralizing utility functions.Let's verify that the new import path is valid and that no other files still import
isNumberfrom the old location:✅ Verification successful
✓ Import change verified and correct
The
isNumberutility is properly exported from@opentiny/utilswith identical implementation, and no files are using the old import path anymore.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the new import path exists and if there are any remaining old imports echo "Checking for the new utility path..." rg -p "export.*isNumber" "@opentiny/utils" echo "Checking for any remaining old imports..." rg "from ['\"]\@opentiny\/vue-renderless\/common\/type['\"].*isNumber"Length of output: 490
Script:
#!/bin/bash # First, find the utils package location echo "Locating utils package..." fd -t f "package.json" | xargs grep -l "@opentiny/utils" # Search for isNumber export in the codebase echo -e "\nSearching for isNumber export..." rg "export.+isNumber" # Check for old imports with fixed regex echo -e "\nChecking for old imports..." rg "from ['\"]@opentiny/vue-renderless/common/type['\"].*isNumber" # Check current usage of isNumber echo -e "\nChecking current isNumber usage..." rg "isNumber\("Length of output: 10086
packages/vue/src/year-table/src/index.ts (1)
2-2: Verify the impact of the date utility function migration.The migration from
isDatetoisDate1appears consistent with the broader refactoring pattern. However, we should verify that the behavior remains identical.Also applies to: 14-14
✅ Verification successful
Migration from
isDatetoisDate1is safeThe change is verified to be safe as
isDate1is an alias export of the originalisDatefunction (isDate as isDate1), ensuring identical behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare implementations of isDate and isDate1 to ensure identical behavior # Search for both old and new implementations to verify consistency echo "Searching for old isDate implementation..." rg -p "function isDate" --type ts echo "Searching for new isDate1 implementation..." rg -p "function isDate1" --type ts echo "Checking for any remaining usages of old isDate..." rg -p "isDate\(" --type tsLength of output: 3772
Script:
#!/bin/bash # Search for isDate1 implementation and related migration patterns echo "Searching for isDate1 implementation..." rg -p "export (const|function) isDate1" --type ts echo "Searching for isDate1 usages..." rg -p "isDate1\(" --type ts echo "Looking for migration documentation..." fd -e md -e txt -X grep -l "isDate.*isDate1" {} echo "Checking for similar migration patterns..." rg -p "import.*@opentiny/utils.*isDate1" --type tsLength of output: 918
Script:
#!/bin/bash # Search specifically in utils package for isDate1 implementation echo "Searching in utils package..." cd packages/utils && rg -p "isDate1" --type ts echo "Checking package.json for utils..." cat packages/utils/package.jsonLength of output: 324
packages/vue/src/date-table/src/index.ts (1)
3-3: LGTM! Consistent with the utility function migration.The validation logic correctly handles both single date values and arrays of dates using the new
isDate1function.Also applies to: 14-14
packages/vue/src/month-table/src/pc.vue (2)
16-16: Good improvement in variable naming.Changed
keyto_keyin the v-for loop to avoid using the reserved word 'key' as a variable name.
29-29: LGTM! Consistent with the utility function migration.The validation logic matches the pattern used in other components, properly handling both single date values and arrays.
Also applies to: 37-37
packages/renderless/src/date-panel/vue.ts (1)
149-149: LGTM!The update from
toDatetotoDate1is consistent with the broader refactoring of date utility functions.packages/utils/src/date-util/fecha.ts (1)
65-65:⚠️ Potential issueFix the regular expression pattern.
The regular expression pattern contains an empty negated character class
[^], which matches any character. Consider using a more specific pattern or the dot (.) character instead.Apply this diff to fix the regex pattern:
-const regexEscape = (str) => str.replace(/[|\\{()[^$+*?.-]/g, '\\$&') +const regexEscape = (str) => str.replace(/[|\\{()[$+*?.-]/g, '\\$&')Likely invalid or redundant comment.
packages/renderless/src/date-range/index.ts (1)
14-25: LGTM! Consistent utility function updates.The import statements have been updated to use the new utility function names (
toDate1,parseDate1,nextMonth1), maintaining consistency with the broader refactoring effort.packages/renderless/src/date-panel/index.ts (1)
16-31: LGTM! Consistent utility function updates.The import statements have been updated to use the new utility function names (
isDate1,parseDate1,nextMonth1), maintaining consistency with the broader refactoring effort.packages/utils/src/index.ts (1)
23-24: Address technical debt in date utilities.The comment indicates pending consolidation of date-related utilities. This technical debt should be addressed:
- Compare functionality between duplicate functions
- Consolidate where possible
- Remove the TODO comment once addressed
Let's analyze the duplicate functions:
| @@ -52,7 +52,7 @@ import { | |||
| computerEnableYearArrow, | |||
| watchPickerVisible | |||
| } from './index' | |||
| import { nextMonth, extractDateFormat, extractTimeFormat } from '../common/deps/date-util' | |||
| import { nextMonth1, extractDateFormat, extractTimeFormat } from '@opentiny/utils' | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration of date utility functions is incomplete
Several components are still using date utilities from the old location ../common/deps/date-util. The migration to @opentiny/utils needs to be completed in:
- packages/renderless/src/year-range/*
- packages/renderless/src/time-range/*
- packages/renderless/src/time/*
- packages/renderless/src/time-spinner/*
- packages/renderless/src/month-range/*
- packages/renderless/src/picker/*
- packages/renderless/src/date-table/*
- packages/renderless/src/calendar/*
- packages/renderless/src/calendar-view/*
- packages/renderless/src/calendar-bar/*
🔗 Analysis chain
Verify complete migration of date utility functions.
While the changes to use nextMonth1 are consistent with the utility function migration, we should verify that all related date utility functions have been migrated.
Also applies to: 98-98
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining unmigrated date utility functions
# Search for imports from the old location and usage of old function names
echo "Checking for remaining imports from old location..."
rg -p "from '\.\./common/deps/date-util'" --type ts
echo "Checking for any remaining usages of old nextMonth..."
rg -p "nextMonth\(" --type ts
echo "Checking for other potential date utility functions that might need migration..."
rg -p "import.*from '\.\./common/deps/date-util'" --type ts
Length of output: 5788
| if (!isDate(dateObj) || isNaN(dateObj.getTime())) { | ||
| throw new Error('Invalid Date in fecha.format') |
There was a problem hiding this comment.
Replace isNaN with Number.isNaN.
Using the global isNaN is unsafe as it performs type coercion. Use Number.isNaN instead for more precise number validation.
Apply this diff to fix the issue:
- if (!isDate(dateObj) || isNaN(dateObj.getTime())) {
+ if (!isDate(dateObj) || Number.isNaN(dateObj.getTime())) {📝 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.
| if (!isDate(dateObj) || isNaN(dateObj.getTime())) { | |
| throw new Error('Invalid Date in fecha.format') | |
| if (!isDate(dateObj) || Number.isNaN(dateObj.getTime())) { | |
| throw new Error('Invalid Date in fecha.format') |
🧰 Tools
🪛 Biome (1.9.4)
[error] 235-235: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
packages/utils/src/index.ts
Outdated
| getI18nSettings, | ||
| isDate as isDate1, | ||
| toDate as toDate1, | ||
| isDateObject, | ||
| formatDate, | ||
| parseDate as parseDate1, | ||
| getDayCountOfMonth, | ||
| getDayCountOfYear, | ||
| getFirstDayOfMonth, | ||
| prevDate, | ||
| nextDate, | ||
| getStartDateOfMonth, | ||
| getWeekNumber, | ||
| getRangeHours, | ||
| range, | ||
| getMonthDays, | ||
| getPrevMonthLastDays, | ||
| getRangeMinutes, | ||
| modifyDate, | ||
| modifyTime, | ||
| modifyWithTimeString, | ||
| clearTime, | ||
| clearMilliseconds, | ||
| limitTimeRange, | ||
| timeWithinRange, | ||
| changeYearMonthAndClampDate, | ||
| nextMonth as nextMonth1, | ||
| prevMonth, | ||
| nextYear, | ||
| prevYear, | ||
| extractTimeFormat, | ||
| extractDateFormat, | ||
| validateRangeInOneMonth | ||
| } from './date-util' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify necessity of duplicate date utility functions.
Several functions are duplicated with numeric suffixes (e.g., isDate1, toDate1, nextMonth1). This increases maintenance burden and may confuse users.
Consider:
- Consolidating duplicate functions if they serve the same purpose
- If different, rename to reflect their specific use cases rather than using numeric suffixes
- Add JSDoc comments to clarify the differences between similar functions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/vue/src/time-picker/index.ts (1)
14-14: LGTM! Consider documenting the utility centralization.The change aligns with the broader effort to centralize utility functions. Consider adding documentation about this architectural change to help other developers understand the new utility organization.
Consider:
- Adding a comment in the README about the centralization of utilities in
@opentiny/utils- Creating a migration guide for teams that might be importing these utilities from the old locations
packages/vue/src/grid-toolbar/src/custom-saas.vue (1)
798-799: Fix formatting in ternary expressions.The ternary expressions need consistent indentation.
Apply this formatting fix:
- ? [this.opt.asc, this.opt.cancelSort] - : [this.opt.asc, this.opt.desc] + ? [this.opt.asc, this.opt.cancelSort] + : [this.opt.asc, this.opt.desc]Also applies to: 804-805, 971-972
🧰 Tools
🪛 ESLint
[error] 798-798: Insert
··(prettier/prettier)
[error] 799-799: Insert
··(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
packages/vue/src/cascader-node/package.json(1 hunks)packages/vue/src/cascader-node/src/pc.vue(1 hunks)packages/vue/src/date-picker/package.json(1 hunks)packages/vue/src/date-picker/src/pc.vue(1 hunks)packages/vue/src/date-table/package.json(1 hunks)packages/vue/src/grid-manager/package.json(1 hunks)packages/vue/src/grid-select/package.json(1 hunks)packages/vue/src/grid-toolbar/package.json(1 hunks)packages/vue/src/grid-toolbar/src/custom-saas.vue(5 hunks)packages/vue/src/grid-toolbar/src/custom.vue(2 hunks)packages/vue/src/grid-toolbar/src/multiple-history.ts(1 hunks)packages/vue/src/grid/src/adapter/src/setup.ts(1 hunks)packages/vue/src/grid/src/cell/src/cell.ts(2 hunks)packages/vue/src/grid/src/fetch-data/src/methods.ts(1 hunks)packages/vue/src/grid/src/filter/src/panel.tsx(2 hunks)packages/vue/src/grid/src/grid/grid.ts(1 hunks)packages/vue/src/grid/src/header/src/header.ts(1 hunks)packages/vue/src/grid/src/keyboard/src/methods.ts(1 hunks)packages/vue/src/grid/src/pager/src/methods.ts(1 hunks)packages/vue/src/grid/src/table/src/methods.ts(2 hunks)packages/vue/src/grid/src/table/src/table.ts(4 hunks)packages/vue/src/grid/src/toolbar/src/methods.ts(1 hunks)packages/vue/src/grid/src/tools/formatter.ts(1 hunks)packages/vue/src/grid/src/validator/src/methods.ts(1 hunks)packages/vue/src/guide/package.json(1 hunks)packages/vue/src/image-viewer/package.json(1 hunks)packages/vue/src/image/package.json(1 hunks)packages/vue/src/input/package.json(1 hunks)packages/vue/src/message/index.ts(1 hunks)packages/vue/src/message/package.json(1 hunks)packages/vue/src/query-builder/package.json(1 hunks)packages/vue/src/query-builder/src/utils/generateID.ts(1 hunks)packages/vue/src/recycle-scroller/package.json(1 hunks)packages/vue/src/time-picker/index.ts(1 hunks)packages/vue/src/time-picker/package.json(1 hunks)packages/vue/src/time-select/index.ts(1 hunks)packages/vue/src/time-select/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/vue/src/grid/src/validator/src/methods.ts
- packages/vue/src/grid/src/grid/grid.ts
- packages/vue/src/grid/src/fetch-data/src/methods.ts
- packages/vue/src/grid/src/cell/src/cell.ts
- packages/vue/src/grid/src/toolbar/src/methods.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/vue/src/grid/src/header/src/header.ts
- packages/vue/src/grid/src/tools/formatter.ts
- packages/vue/src/grid/src/table/src/table.ts
- packages/vue/src/grid/src/table/src/methods.ts
🧰 Additional context used
🪛 ESLint
packages/vue/src/grid-toolbar/src/custom-saas.vue
[error] 798-798: Insert ··
(prettier/prettier)
[error] 799-799: Insert ··
(prettier/prettier)
[error] 804-804: Insert ··
(prettier/prettier)
[error] 805-805: Insert ··
(prettier/prettier)
[error] 971-971: Insert ··
(prettier/prettier)
[error] 972-972: Insert ··
(prettier/prettier)
🔇 Additional comments (70)
packages/vue/src/grid-toolbar/package.json (1)
15-15: New Dependency Addition on "@opentiny/utils":
The addition of"@opentiny/utils": "workspace:~",integrates the updated utilities into the grid-toolbar package. Please ensure that this workspace reference correctly resolves to the intended version of@opentiny/utilsand remains consistent with your workspace’s versioning strategy. It would also be good to verify that any changes in the utils package (especially around the updated SHA256 functionality and environment detection replacements) are fully compatible with the grid-toolbar package usage.packages/vue/src/time-picker/package.json (4)
3-3: ESM Module Declaration AddedAdding
"type": "module"explicitly defines the package as an ECMAScript module. This change improves inter-package consistency and ensures consumers interpret the module format correctly.
6-7: License & Side-Effects Specification IntroducedThe inclusion of
"license": "MIT"clarifies the legal usage, and"sideEffects": falsehelps bundlers optimize for tree shaking. Both are valuable enhancements for package clarity and performance.
15-19: Dependency Reorganization and AdditionThe dependencies section now:
- Adds
"@opentiny/utils": "workspace:~"to centralize utility functions.- Reintroduces and reorganizes
"@opentiny/vue-common","@opentiny/vue-renderless", and"@opentiny/vue-theme".Please verify that:
- The versions specified with workspace specifiers align with the overall monorepo versioning.
- All internal references in the time-picker code are updated to reflect any necessary changes arising from this reorganization.
21-24: DevDependencies Reintroduced for TestingThe reintroduction of
"@opentiny-internal/vue-test-utils"and"vitest"helps reestablish the testing framework for the package. Ensure that these tools are properly configured and that tests are added/updated as required by the PR checklist.packages/vue/src/recycle-scroller/package.json (5)
3-3: Explicit Module Declaration Added.
The addition of"type": "module"explicitly declares the package as an ECMAScript module, which improves clarity for consumers and bundlers.
6-6: License Specification Added.
Including"license": "MIT"clearly communicates the licensing for the package, ensuring consistency across the project.
7-7: Side Effects Optimization Declared.
Setting"sideEffects": falseinforms bundlers that the module does not include side effects, which can improve tree-shaking and overall bundle optimization.
11-11: Workspace Dependency on @opentiny/utils.
Adding"@opentiny/utils": "workspace:~"as a dependency aligns with the project’s workspace strategy. Please verify that the version resolution syntax (workspace:~) is intentional and consistent with other packages in the monorepo.
16-19: Restructured DevDependencies Section.
The reorganization of the"devDependencies"block, including the addition of"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:", improves clarity. Ensure that the version alias"catalog:"for"vitest"is correct and intentional, as it deviates from the more common semantic versioning patterns seen elsewhere.packages/vue/src/guide/package.json (4)
3-3: Addition of "type": "module" Field
The introduction of"type": "module"clearly signals that this package uses ECMAScript modules. This is important for proper module resolution and aligns with modern JavaScript practices.
6-7: Inclusion of Licensing and Side Effects Metadata
Adding"license": "MIT"provides clear licensing information, and"sideEffects": falsehelps bundlers optimize imports. Both fields improve package metadata and are aligned with good practices.
12-16: Updating and Restoring Dependency Entries
The dependencies block now includes:
"@opentiny/utils": "workspace:~""@opentiny/vue-common": "workspace:~""@opentiny/vue-renderless": "workspace:~""@opentiny/vue-theme": "workspace:~"- Additionally,
"shepherd.js": "11.1.1"has been added.These changes restore several dependencies and add new ones, ensuring that all required packages are available. Please verify that the workspace versions (using
"workspace:~") resolve as intended within your monorepo setup.
18-21: Reintroducing Testing Utilities in DevDependencies
The"devDependencies"section now includes:
"@opentiny-internal/vue-test-utils": "workspace:*""vitest": "catalog:"This reintroduction aligns with similar changes in other packages and ensures that robust testing utilities are available for development. Confirm that these versions match your monorepo’s overall testing configuration and that
"vitest": "catalog:"is correctly interpreted by your dependency manager.packages/vue/src/query-builder/package.json (2)
3-3: Module Type, Licensing, and Side Effects Fields Added
The additions of"type": "module","license": "MIT", and"sideEffects": falsehelp modernize the package configuration and clarify its module behavior and licensing. These are all in line with current best practices.Also applies to: 6-7
31-34: Reintroduced DevDependencies Block – Verify Specifiers
The addition of the devDependencies section—with"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:"—indicates a move towards reintroducing internal testing utilities. Please confirm that the version specifier"catalog:"for Vitest is intentional and correctly resolved in your workspace configuration.packages/vue/src/query-builder/src/utils/generateID.ts (2)
3-51: Well-implemented UUID generation with robust fallback strategy.The implementation is excellent with several notable strengths:
- Prioritizes secure
crypto.randomUUID()when available- Falls back to
crypto.getRandomValues()with proper v4 UUID formatting- Provides a last-resort implementation using
random()- Includes comprehensive documentation and clear code comments
1-1: Verify the compatibility of the importedrandomfunction.The import source has been changed from
@opentiny/vue-renderless/common/stringto@opentiny/utils. Please ensure that therandomfunction from the new source maintains the same interface and behavior.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Compare the implementation of random function in both locations # Search for the old implementation echo "Old implementation:" rg -A 5 "export.*random.*=" "packages/vue-renderless/common/string" # Search for the new implementation echo "New implementation:" rg -A 5 "export.*random.*=" "packages/utils"Length of output: 662
UUID Fallback Strategy: Validate
randomCompatibilityThe output confirms that the new
randomfunction is exported frompackages/utils/src/string/index.tsand its implementation usingcrypto.getRandomValuesmatches the expected interface for generating UUIDs. There is no indication of any interface or behavioral discrepancies with the previous version.packages/vue/src/grid-manager/package.json (4)
3-3: ES Module Declaration Added
The addition of"type": "module"ensures that the package is correctly interpreted as using ES modules.
6-7: License and Side Effects Metadata Introduced
Including"license": "MIT"and"sideEffects": falseclarifies the package’s licensing and informs bundlers that there are no side effects on import.
15-15: Centralizing Utility Dependency
Adding the dependency"@opentiny/utils": "workspace:~"promotes consistency by centralizing shared utility functions across packages.
18-21: Reintroduction of Testing Utilities
Restoring the devDependencies with"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:"ensures that the testing tools are uniformly available.packages/vue/src/message/package.json (4)
3-3: ES Module Declaration Added
The new"type": "module"field ensures compatibility with ES module imports throughout the package.
6-7: License and Side Effects Fields Added
Introducing"license": "MIT"and"sideEffects": falseimproves package metadata clarity and optimizes bundler behavior.
10-13: Addition of Build Scripts
The newly added"scripts"block standardizes the build process usingpnpm, which enhances consistency across the project’s packages.
15-15: Utility Dependency Inclusion
Adding"@opentiny/utils": "workspace:~"as a dependency streamlines the usage of common utilities across the codebase.packages/vue/src/date-table/package.json (4)
3-3: ES Module Declaration Introduced
The inclusion of"type": "module"enables ES module support, enhancing interoperability.
6-7: Metadata Update for Licensing and Build Optimization
By adding"license": "MIT"and"sideEffects": false, the package now documents its licensing and assures bundlers about side-effect-free imports.
15-15: Utility Dependency Standardization
The addition of"@opentiny/utils": "workspace:~"aligns this package with the centralized approach for utility functions.
20-23: Reinstatement of Testing Dependencies
Reintroducing the devDependencies—"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:"—ensures that testing capabilities remain consistent with other packages.packages/vue/src/grid-select/package.json (4)
3-3: ES Module Declaration Added
Adding"type": "module"ensures that consumers of the package get proper ES module semantics.
6-7: License and Side Effects Specification Added
The new"license": "MIT"and"sideEffects": falseentries provide clear metadata and assist in optimizing module bundling.
15-19: Updated Dependencies for Consistency
The updated dependencies—including the new"@opentiny/utils": "workspace:~"and reorganized entries like"@opentiny/vue-common","@opentiny/vue-grid", and"@opentiny/vue-renderless"—support a unified dependency management strategy.
21-24: Reintroduction of DevDependencies
Restoring the devDependencies with"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:"maintains consistency with testing support across packages.packages/vue/src/image/package.json (5)
3-3: ES Module Declaration Introduced
The inclusion of"type": "module"correctly designates this package as an ES module.
6-7: License and Side Effects Fields Added
Adding"license": "MIT"and"sideEffects": falseimproves metadata clarity and informs bundlers that the module is free of side effects.
15-16: Utility Dependency Inclusion
The addition of"@opentiny/utils": "workspace:~"and the retention of"@opentiny/vue-common": "workspace:~"reinforce the move toward centralized utilities.
17-20: Verify Dependency Placement for Vue Icon and Theme
The ai-summary indicates that"@opentiny/vue-icon"and"@opentiny/vue-theme"should be moved from dependencies to devDependencies. Please verify that their current placement in the"dependencies"section aligns with their intended runtime usage.
22-25: Consistent DevDependencies Configuration
Restoring the testing dependencies"@opentiny-internal/vue-test-utils": "workspace:*"and"vitest": "catalog:"ensures that the package continues to support a standardized testing approach across the project.packages/vue/src/date-picker/package.json (4)
3-3: ECMAScript Module DeclarationAdding
"type": "module"clearly indicates that this package is using ES modules. This change improves interoperability and helps bundlers correctly interpret the module format.
6-7: License and Side Effects MetadataThe new
"license": "MIT"and"sideEffects": falsefields enhance clarity regarding licensing and provide hints to bundlers for tree-shaking, respectively.
15-21: Dependencies Reorganization and UpdatesReintroducing and reordering dependencies (such as
"@opentiny/utils","@opentiny/vue-input","@opentiny/vue-picker", and"@opentiny/vue-renderless") aligns with the broader repository restructuring. Please verify that the version labels (usingworkspace:~) are consistent and compatible with other packages.
23-26: DevDependencies ReintroductionThe readdition of
"@opentiny-internal/vue-test-utils"and"vitest"supports improved testing practices. Confirm that these versions match the expectations across the project.packages/vue/src/cascader-node/package.json (4)
3-3: Module Format StandardizationAdding
"type": "module"ensures consistency with the repository’s shift toward modern ECMAScript modules, which simplifies integration with current tooling.
6-7: License and Side Effects DeclarationIntroducing
"license": "MIT"and"sideEffects": falseprovides both legal clarity and bundler optimization hints.
15-21: Updated DependenciesThe dependencies section now includes
"@opentiny/utils"and repositions packages such as"@opentiny/vue-common","@opentiny/vue-radio", and"@opentiny/vue-renderless". This reorganization reflects the updated dependency management strategy. Please double-check that these updates are harmonized with other modules.
23-26: DevDependencies ConfigurationReintroducing
"@opentiny-internal/vue-test-utils"and"vitest"reestablishes the testing foundation for this package.packages/vue/src/input/package.json (4)
3-3: ES Module DeclarationThe addition of
"type": "module"is appropriate for enabling ECMAScript module support, aligning this package with modern JavaScript standards.
6-7: License and Side Effects ClarificationBy adding
"license": "MIT"and"sideEffects": false, the package now provides important metadata for legal clarity and optimization during bundling.
15-24: Dependencies Update and ReorderingThe dependencies have been restructured to include
"@opentiny/utils"and reorganized packages such as"@opentiny/vue-common","@opentiny/vue-icon", and"@opentiny/vue-tooltip". This update supports a standardized configuration across packages. Please verify that the dependency versions and workspace notations are consistent with the overall project.
25-28: Reintroducing DevDependenciesThe reinstated devDependencies—
"@opentiny-internal/vue-test-utils"and"vitest"—ensure that adequate testing support is maintained throughout the package.packages/vue/src/image-viewer/package.json (4)
3-3: ES Module Format ConfirmationIncluding
"type": "module"confirms the package’s use of modern module semantics, aiding both developers and build tools.
6-7: Legal and Optimization MetadataThe new
"license": "MIT"and"sideEffects": falsesettings ensure legal compliance and improve the bundling process by signaling that there are no unexpected side effects.
15-23: Comprehensive Dependencies UpdateThe dependencies section now features additions such as
"@opentiny/utils","@opentiny/vue-action-sheet","@opentiny/vue-dropdown-menu","@opentiny/vue-icon", and"@opentiny/vue-renderless", along with reordering of existing dependencies. This consolidates dependency management across the project—please verify that all versions are aligned with project standards.
25-28: Standardized DevDependenciesRestoring
"@opentiny-internal/vue-test-utils"and"vitest"enhances development and testing consistency.packages/vue/src/cascader-node/src/pc.vue (1)
94-96: LGTM! Proper handling of object comparison.The use of
isEqualfor deep object comparison is correct, and the reassignment of the reference when values are equal is a good practice for Vue's reactivity system.packages/vue/src/message/index.ts (1)
2-2: Verify compatibility of the new extend implementation.The import source for
extendhas been changed as part of utility function centralization. Please ensure that the new implementation in@opentiny/utilsmaintains the same deep merge behavior required for component extension.packages/vue/src/date-picker/src/pc.vue (1)
15-15: LGTM! Consistent with utility centralization.The change aligns with the broader effort to centralize utility functions.
packages/vue/src/time-select/index.ts (1)
13-13: LGTM! Consistent with utility centralization.The change aligns with the broader effort to centralize utility functions.
packages/vue/src/grid/src/adapter/src/setup.ts (1)
25-25: Verify the compatibility of the new extend implementation.The
extendfunction is used for merging configuration options. Ensure that the implementation from@opentiny/utilsmaintains the same deep merge behavior as the previous version.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the new extend implementation maintains the same behavior # Search for tests or usage patterns of extend function # Look for test files related to extend fd -e test.ts -e spec.ts -e test.js -e spec.js | xargs rg "extend.*@opentiny/utils" # Look for other usages of extend with similar patterns rg "extend\(true, \{.*\}, .*\)" -A 2Length of output: 5815
Deep Merge Behavior Confirmed
All instances using
extend(true, ..., ...)across the codebase—including in configuration merging in grid, toolbar, picker, and other components—show that the new implementation from@opentiny/utilsis used in the same deep merge pattern as before. This indicates the new implementation maintains the required deep merge behavior, ensuring compatibility in critical configuration operations.packages/vue/src/grid-toolbar/src/multiple-history.ts (1)
14-14: Verify array merging behavior with the new extend implementation.The
extendfunction is used inmergeArrayfor combining array items. Ensure that the implementation from@opentiny/utilsmaintains the same array merging behavior.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the new extend implementation maintains array merging behavior # Search for tests or similar usage patterns # Look for test files related to mergeArray fd -e test.ts -e spec.ts -e test.js -e spec.js | xargs rg "mergeArray.*extend" # Look for similar array merging patterns rg "extend\(true, \{\}, .*\[\].*\)" -A 2Length of output: 123
Let's gather a bit more context on how the array merge is implemented in the file. We'll inspect the usage of both the
mergeArrayfunction and theextendcall in the relevant file to determine if the new implementation produces the expected behavior.
🏁 Script executed:
#!/bin/bash # Check for the mergeArray function and its surrounding context in the file. rg -n "mergeArray" packages/vue/src/grid-toolbar/src/multiple-history.ts -C 8 # Check the usage of extend in the same file to see how it's applied. rg -n "extend(" packages/vue/src/grid-toolbar/src/multiple-history.ts -C 8Length of output: 1102
Let's run another script with the proper escaping to check the usage of
extendin the file.
🏁 Script executed:
#!/bin/bash # Check for usages of extend in the file with properly escaped parenthesis rg -n "extend\(" packages/vue/src/grid-toolbar/src/multiple-history.ts -C 8Length of output: 406
Array Merging Behavior Review:
The implementation inmergeArraycallsresult.push(extend(true, {}, toItem || {}, fromItem))which indicates a deep merge of the source objects. Based on our code inspection, the usage of the new
extendfrom@opentiny/utilsappears consistent with the intended behavior (merging properties, including recursively merging nestedchildrenarrays). Although no dedicated unit tests formergeArraywere located via our search, the merge logic itself is clear and in line with expected deep merge patterns. It is recommended that manual or additional automated tests be run to exercise edge cases (e.g., handling of nested objects inchildren) to fully validate the behavior in practical grid scenarios.packages/vue/src/grid/src/pager/src/methods.ts (1)
5-5: Verify shallow copy behavior with the new extend implementation.The
extendfunction is used withfalseparameter inbeforePageChangeHandler, indicating a shallow copy operation. Ensure that the implementation from@opentiny/utilsmaintains the same shallow copy behavior.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the new extend implementation maintains shallow copy behavior # Search for tests or similar usage patterns # Look for test files related to shallow copy fd -e test.ts -e spec.ts -e test.js -e spec.js | xargs rg "extend\(false," # Look for similar shallow copy patterns rg "extend\(false, .*\)" -A 2Length of output: 878
Shallow Copy Behavior Verified
The usage of
extendwith thefalseflag in bothpackages/vue/src/grid/src/grid/grid.tsandpackages/vue/src/grid/src/pager/src/methods.tsconfirms that the shallow copy behavior is maintained consistently with the new implementation from@opentiny/utils.
- Verify that the same shallow copy pattern (
extend(false, { $grid: this }, params)) is used across relevant files.packages/vue/src/grid-toolbar/src/custom.vue (1)
181-181: Verify deep merge behavior in Vue computed context.The
extendfunction is used withtrueparameter in thehistoryConfigcomputed property. Ensure that the implementation from@opentiny/utilsmaintains the same deep merge behavior and reactivity in Vue context.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the new extend implementation maintains deep merge behavior in Vue context # Search for tests or similar usage patterns # Look for test files related to Vue computed properties using extend fd -e test.ts -e spec.ts -e test.js -e spec.js | xargs rg "extend.*computed" # Look for similar Vue computed patterns rg "extend\(true, \{\}, .*computed" -A 2Length of output: 121
🏁 Script executed:
#!/bin/bash # Extract context around the computed property "historyConfig" in the custom.vue file to verify deep merge usage rg "historyConfig" packages/vue/src/grid-toolbar/src/custom.vue -C 10Length of output: 2502
Deep merge behavior in Vue computed property verified.
The computed property
historyConfigusesextend(true, {}, ...)for deep merging. The implementation inpackages/vue/src/grid-toolbar/src/custom.vuecorrectly follows the pattern of a deep merge, which should maintain Vue reactivity as the computed property recomputes based on its dependencies.packages/vue/src/grid/src/keyboard/src/methods.ts (1)
28-28: LGTM! Import statement updated correctly.The change to import
extendfrom@opentiny/utilsaligns with the broader refactoring effort to centralize utility functions.packages/vue/src/grid/src/filter/src/panel.tsx (2)
32-32: LGTM! Import statement updated correctly.The change to import
extendfrom@opentiny/utilsaligns with the broader refactoring effort to centralize utility functions.
1-1: Review computed property usage.The ESLint disable comment suggests that computed properties might be used as methods. Consider refactoring these instances to proper computed properties or methods based on their usage.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for computed properties used as methods ast-grep --pattern 'computed: { $$$ $name() { $$$ } $$$ }'Length of output: 72
I'll run a new script to list any computed property definitions in this file for further context. Please run the script below to show a few lines around any
"computed:"declarations inpackages/vue/src/grid/src/filter/src/panel.tsx.
🏁 Script executed:
#!/bin/bash # Display computed properties in panel.tsx for further investigation rg -n --context 5 "computed:" packages/vue/src/grid/src/filter/src/panel.tsxLength of output: 312
Computed properties appear correctly defined.
The computed block in
packages/vue/src/grid/src/filter/src/panel.tsxdefines theconditionandvisiblegetters properly, and our search did not reveal any instances where they’re invoked as methods (i.e. with parentheses). If their usage elsewhere in the file also accesses them as simple properties, the ESLint disable comment may no longer be necessary.packages/vue/src/grid-toolbar/src/custom-saas.vue (3)
291-295: LGTM! Improved variable naming.Renaming the loop variable from 'column' to 'columnLi' enhances code clarity by avoiding variable shadowing with the parent scope.
Also applies to: 303-303
388-388: LGTM! Import statement updated correctly.The change to import
extendfrom@opentiny/utilsaligns with the broader refactoring effort to centralize utility functions.
444-444: LGTM! Added explicit emits declaration.Adding explicit emits improves the component contract by clearly documenting the events that can be emitted.
packages/vue/src/time-select/package.json (1)
3-7: LGTM! Package configuration improved.The changes enhance the package configuration by:
- Declaring ECMAScript module type
- Adding proper MIT license
- Adding performance optimization hint with sideEffects
- Adding @opentiny/utils dependency
- Adding necessary test utilities
Also applies to: 15-19, 21-24
| @@ -14,7 +14,7 @@ import { renderless, api } from '@opentiny/vue-renderless/cascader-node/vue' | |||
| import { $prefix, setup, h, $props, defineComponent } from '@opentiny/vue-common' | |||
| import Checkbox from '@opentiny/vue-checkbox' | |||
| import Radio from '@opentiny/vue-radio' | |||
| import { isEqual } from '@opentiny/vue-renderless/common/object' | |||
| import { isEqual } from '@opentiny/utils' | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Verify package availability.
The consolidation of utility functions into @opentiny/utils improves code organization and maintainability.
Run the following script to verify the package and function availability:
🏁 Script executed:
#!/bin/bash
# Description: Verify the availability of isEqual in @opentiny/utils
# Check if the package is installed and accessible
npm list @opentiny/utils
# Search for isEqual exports
ast-grep --pattern 'export { $_ as isEqual }'Length of output: 126
Action Required: @opentiny/utils is Not Properly Available
The shell script results indicate:
- The dependency check with
npm list @opentiny/utilsshows no installed instance. - The AST grep did not locate any export of an
isEqualfunction from the package.
This suggests that the intended utility function is either missing from the package or not properly exported, which may lead to runtime errors.
- Verify that
@opentiny/utilsis added as a dependency in your project. - Confirm that the
isEqualfunction is indeed exported from@opentiny/utils(or adjust the import if it has been renamed or relocated).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/utils/src/index.ts (1)
24-58: 🛠️ Refactor suggestionRefactor duplicate date utility functions.
Several functions are duplicated with numeric suffixes (e.g.,
isDate1,toDate1,nextMonth1). This pattern:
- Increases maintenance burden
- May confuse users about which function to use
- Violates the DRY principle
Consider:
- Consolidating duplicate functions if they serve the same purpose
- If different, rename to reflect their specific use cases rather than using numeric suffixes
- Add JSDoc comments to clarify the differences between similar functions
🧹 Nitpick comments (1)
packages/utils/src/index.ts (1)
111-112: Remove redundant comments.The comments "无直接引用" (no direct reference) don't provide meaningful information and should be removed or replaced with more descriptive comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/renderless/src/base-select/index.ts(3 hunks)packages/renderless/src/grid/utils/common.ts(1 hunks)packages/renderless/src/nav-menu/index.ts(2 hunks)packages/renderless/src/popeditor/index.ts(1 hunks)packages/renderless/src/select/index.ts(3 hunks)packages/renderless/src/tree-select/index.ts(1 hunks)packages/renderless/src/tree/index.ts(1 hunks)packages/utils/src/index.ts(3 hunks)packages/vue/src/grid/src/fetch-data/src/methods.ts(1 hunks)packages/vue/src/grid/src/filter/src/methods.ts(1 hunks)packages/vue/src/scrollbar/package.json(1 hunks)packages/vue/src/scrollbar/src/pc.vue(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/renderless/src/tree/index.ts
- packages/renderless/src/tree-select/index.ts
- packages/vue/src/grid/src/filter/src/methods.ts
- packages/renderless/src/popeditor/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/grid/src/fetch-data/src/methods.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/renderless/src/base-select/index.ts
[error] 1866-1868: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
packages/vue/src/scrollbar/package.json (6)
3-3: Addition of Module Type DeclarationThe
"type": "module"field has been added to indicate that this package uses ECMAScript modules. This is appropriate if your codebase and build tools are set up for ESM.
6-6: License Field DeclarationAdding the
"license": "MIT"field clarifies the licensing of this package. This explicit declaration is beneficial for open source compliance and clarity.
7-7: Declaration of Side EffectsThe addition of
"sideEffects": falseinforms bundlers that this package is free from side effects, potentially enabling better tree-shaking. Verify that none of the module files introduce unintended side effects before confirming.
15-15: Centralized Utility Dependency AdditionIncluding
"@opentiny/utils": "workspace:~"in the dependencies aligns with the broader effort to centralize utility functions across packages. Ensure the version alignment is consistent with your workspace settings.
17-17: Update to vue-renderless DependencyThe update for
"@opentiny/vue-renderless": "workspace:~"standardizes the dependency version within your workspace. Double-check that this change does not break compatibility with other packages relying on vue-renderless.
20-23: Reorganization of DevDependenciesThe devDependencies block now includes:
"@opentiny-internal/vue-test-utils": "workspace:*""vitest": "catalog:"This reordering aligns with project testing standards and dependency management practices. Verify that the new versions and specifiers work seamlessly with your CI and local test environment.
packages/vue/src/scrollbar/src/pc.vue (1)
16-16: LGTM! Import path updated correctly.The import path change aligns with the centralization of utility functions under
@opentiny/utils.packages/utils/src/index.ts (1)
23-23: Address the TODO comment about merging date utilities.The comment indicates that
date.tsanddate-util.tsneed to be merged. This should be prioritized to reduce code duplication and maintenance overhead.Would you like me to help create an issue to track the merging of these date utility files?
packages/renderless/src/grid/utils/common.ts (1)
27-27: LGTM! Import path updated correctly.The import path change aligns with the centralization of utility functions under
@opentiny/utils.packages/renderless/src/nav-menu/index.ts (2)
24-24: LGTM! Import paths updated correctly.The import path changes align with the centralization of utility functions under
@opentiny/utils.Also applies to: 27-27
494-494: Verify the impact of switching fromisBrowsertoisServercheck.The environment check has been inverted from
!isBrowsertoisServer. While semantically equivalent, this is a breaking change that might affect other parts of the codebase.Run the following script to check for other occurrences of this pattern:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for other instances of isBrowser to isServer conversion # Search for remaining isBrowser checks echo "Checking for remaining isBrowser checks..." rg "isBrowser|!isBrowser" || echo "No isBrowser checks found" # Search for isServer checks echo -e "\nChecking for isServer checks..." rg "isServer" || echo "No isServer checks found"Length of output: 15750
Environment Check Conversion Verified
The change from using
!isBrowsertoisServeris consistent with similar patterns throughout the codebase. In our search results, many modules already use theisServercheck. SinceisServeris defined as the inverse ofisBrowser(i.e.isServeris true whenwindowis undefined), this inversion is semantically equivalent and does not introduce any breaking behavior.packages/renderless/src/base-select/index.ts (2)
1-1: LGTM! Improved imports organization.Centralizing utility imports from
@opentiny/utilsimproves maintainability and consistency.Also applies to: 7-7, 19-19
1868-1868: LGTM! Improved server-side rendering check.Changed from
!isBrowsertoisServerfor a more explicit server-side rendering check.🧰 Tools
🪛 Biome (1.9.4)
[error] 1866-1868: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/select/index.ts (2)
1-1: LGTM! Improved imports organization.Centralizing utility imports from
@opentiny/utilsimproves maintainability and consistency.Also applies to: 7-7, 19-19
2401-2401: LGTM! Improved server-side rendering check.Changed from
!isBrowsertoisServerfor a more explicit server-side rendering check.
shenjunjian
changed the title
shenjunjian
changed the title
PR
删除 renderless/common的目录，并同时调整所有的引用路径
原来renderless/common/runtime.ts 没有了
所有公共函数都从 @opentiny/utils vue-directive vue-hooks 三个包中来
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
Dependency Removal
js-sha256dependency from Nuxt, Vue2, and Vue3 example projects.Refactoring
sha256from@opentiny/utils.getWindowandisWebutility functions.globalThis.isServerfrom@opentiny/utils.@opentiny/utils.@opentiny/utils.New Features
fecha, for comprehensive date manipulation.selectcomponent for enhanced functionality.Exports
sha256function.Popperclass and added new properties for improved functionality.