feat(utils): extract the common methods in the dom.gloabl.type files to the untils package #2823
Conversation
wuyiping0628
added
documentation
WalkthroughThe pull request introduces a comprehensive refactoring of utility function imports across multiple packages in the OpenTiny project. The primary change involves transitioning from local utility modules to a centralized Changes
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 (
|
wuyiping0628
changed the title
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
WalkthroughThis pull request refactors the codebase by extracting utility functions from the Changes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (22)
packages/renderless/src/common/validate/rules/required.ts (1)
14-14: Consider using a more descriptive parameter name.While renaming the
typeparameter to_typeresolves the naming conflict with the importedtypeutility, consider using a more descriptive name likevalueTypeorfieldTypeto better convey its purpose.-export default function ({ rule, checkValue, source, errors, options, _type }) { - if (rule.required && (!type.hasOwn.call(source, rule.field) || util.isEmptyValue(checkValue, type || rule._type))) { +export default function ({ rule, checkValue, source, errors, options, valueType }) { + if (rule.required && (!type.hasOwn.call(source, rule.field) || util.isEmptyValue(checkValue, type || rule.valueType))) {Also applies to: 16-17
packages/renderless/src/common/validate/validations/integer.ts (2)
15-15: LGTM! Consider consolidating empty value checks.The changes align with the utility migration pattern. Note that this validation has an additional empty string check (
checkValue !== '') that could potentially be consolidated withisEmptyValue()for consistency across validators.Also applies to: 19-19
15-21: Architectural Improvement: Successful utility centralization.The consistent migration of type utilities from local implementations to @opentiny/utils across all validation files:
- Improves maintainability by centralizing type-checking logic
- Reduces code duplication
- Makes it easier to update and optimize type-checking implementations in the future
This refactoring aligns perfectly with the PR's objective of extracting functions from type to the utils package.
packages/renderless/src/common/validate/validations/string.ts (1)
15-15: LGTM! Completes the consistent refactoring pattern.The changes successfully complete the pattern of centralizing type utilities while preserving all string-specific validation logic.
This refactoring improves maintainability by:
- Centralizing type utilities in @opentiny/utils
- Reducing duplication across validation files
- Making future type-related changes easier to manage
Also applies to: 19-19
packages/renderless/src/grid/static/base/helperEqualCompare.ts (1)
Line range hint
28-31: Consider moving all type checks to @opentiny/utils for consistency.Currently, some type checks (isNumber, isString, isBoolean) are local while others (isRegExp, isDate) use @opentiny/utils. Consider moving all type checks to @opentiny/utils for better maintainability and consistency.
-import isNumber from './isNumber' -import isString from './isString' -import isBoolean from './isBoolean' -import isUndefined from './isUndefined'Then update the usage:
-if (val1 && val2 && !isNumber(val1) && !isNumber(val2) && !isString(val1) && !isString(val2)) { +if (val1 && val2 && !type.isNumber(val1) && !type.isNumber(val2) && !type.isString(val1) && !type.isString(val2)) {packages/renderless/src/chart-sankey/index.ts (1)
51-51: Consider simplifying the conditional logic.The current implementation could be made more readable by extracting the condition into a separate variable or helper function.
- mapHandler = (link) => (type.isNull(link.value) ? { ...link, value: dataMap[link.target] } : link) + mapHandler = (link) => { + const shouldUseTargetValue = type.isNull(link.value); + return shouldUseTargetValue ? { ...link, value: dataMap[link.target] } : link; + }packages/renderless/src/container/index.ts (1)
30-30: Consider adding TypeScript type annotations for better type safety.The
transferWidthOrHeightfunction could benefit from explicit type annotations to improve maintainability and catch potential type-related issues early.-const transferWidthOrHeight = (value) => (type.isNumber(value) ? value + 'px' : value) +const transferWidthOrHeight = (value: number | string): string => (type.isNumber(value) ? value + 'px' : value)packages/renderless/src/common/dataset/index.ts (1)
Line range hint
112-132: Enhance error handling in the Promise chain.The Promise chain in the
source.urlblock could be improved to handle errors more gracefully and provide better error feedback.Consider applying this improvement:
if (type.isObject(source) && source.url) { const { type = 'GET', data, beforeRequest, afterRequest, success, hideErr, url, method, ...options } = source options.url = url options.method = method || type.toLocaleLowerCase() const mergeTarget = options.method === 'get' ? 'params' : 'data' options[mergeTarget] = data || {} const afterRequestFn = afterRequest || success const config = { ...options } handlerArgs(config, args) beforeRequest && beforeRequest(config, args) $service.network .request(config) .then((response) => { afterRequestFn && afterRequestFn(response.data) resolve(transForm(response.data, tree)) }) .catch((error) => { - hideErr || reject(error) + if (!hideErr) { + const enhancedError = { + originalError: error, + request: config, + timestamp: new Date().toISOString() + } + reject(enhancedError) + } })packages/vue-locale/src/glob.ts (1)
Line range hint
97-114: Simplify date formatting logic and improve null handling.The date formatting logic has multiple nested conditions and type checks that could be simplified.
Consider this more concise implementation:
- if (type.isNull(value)) { - return value - } - - let date = type.isDate(value) ? value : toDate(value) - let dbtimezone = opt.DbTimezone - let includeTz = value.match && value.match(TZRE) - const convers = format === false || arguments[2] === false - - if (includeTz) { - dbtimezone = getStrTimezone(value) - date = toDate(value.replace('T', ' ').slice(0, -5)) - } - if (!convers) { - date = this.getDateWithNewTimezone(date, dbtimezone, opt.Timezone, opt.TimezoneOffset) - } - - return type.isDate(date) ? formatDate(date, format || opt.DateFormat, t) : null + if (type.isNull(value)) return value + + const parseDate = () => { + if (type.isDate(value)) return value + + const includeTz = value.match?.(TZRE) + if (!includeTz) return toDate(value) + + return toDate(value.replace('T', ' ').slice(0, -5)) + } + + const date = parseDate() + if (!type.isDate(date)) return null + + const dbtimezone = value.match?.(TZRE) ? getStrTimezone(value) : opt.DbTimezone + const shouldConvert = format !== false && arguments[2] !== false + + const finalDate = shouldConvert + ? this.getDateWithNewTimezone(date, dbtimezone, opt.Timezone, opt.TimezoneOffset) + : date + + return formatDate(finalDate, format || opt.DateFormat, t)🧰 Tools
🪛 Biome (1.9.4)
[error] 103-104: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 eslint
[error] 104-104: Use the rest parameters instead of 'arguments'.
(prefer-rest-params)
packages/renderless/src/common/validate/rules/type.ts (1)
Line range hint
19-23: Pre-compile regex patterns for better performance.The email regex pattern is constructed on every module load. Consider pre-compiling it.
-const emailReg1 = '^(([^<>()\\[\\]\\\\.,;:\\s@"]+(\\.[^<>()\\[\\]\\\\.,;:\\s@"]+)*)|(".+"))' -const emailReg = new RegExp( - emailReg1 + '@((\\[[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}\\.[0-9]{1,3}])|(([a-zA-Z\\-0-9]+\\.)+[a-zA-Z]{2,6}))$' -) +const EMAIL_PATTERN = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,6}))$/packages/renderless/src/wizard/index.ts (1)
Line range hint
78-85: Improve cloneDeep implementation.The current implementation of
cloneDeepcould be simplified and made more robust.-export const cloneDeep = (data) => { - if (type.isObject(data)) { - return extend(true, data) - } else if (Array.isArray(data)) { - return copyArray(data) - } else { - return data - } -} +export const cloneDeep = (data) => { + if (data === null || typeof data !== 'object') { + return data + } + + return Array.isArray(data) + ? copyArray(data) + : extend(true, data) +}packages/utils/src/type/index.ts (1)
150-150: LGTM! Consider improving type annotations further.The addition of
anytype annotations to parameters is a good start for type safety. However, consider using more specific types or generic constraints where possible to provide better type checking.Consider using more specific return types and generic constraints:
-export const isDate = (value: any) => typeOf(value) === 'date' +export const isDate = (value: any): value is Date => typeOf(value) === 'date' -export const isPromise = (val: any) => isObject(val) && isFunction(val.then) && isFunction(val.catch) +export const isPromise = <T = any>(val: any): val is Promise<T> => isObject(val) && isFunction(val.then) && isFunction(val.catch)Also applies to: 166-166
packages/renderless/src/cascader-panel/node.ts (1)
83-83: Verify the logic inversion in isLeaf getter.The change from
!isNull(leafValue)to!type.isNull(leafValue)maintains the same logical outcome, but the code readability could be improved.Consider simplifying the logic to make it more readable:
- const isLeaf = !type.isNull(leafValue) ? leafValue : loaded ? !children.length : false + const hasLeafValue = !type.isNull(leafValue) + const isLeaf = hasLeafValue ? leafValue : loaded ? !children.length : falsepackages/renderless/src/checkbox/index.ts (1)
212-212: Simplify the null checks incomputedShowText.The logic in
computedShowTextcan be simplified to avoid redundant null checks.- if (props.text || !type.isNull(props.text)) { + if (!type.isNull(props.text)) { return props.text } else { return props.label }Also applies to: 217-217
packages/renderless/src/chart-candle/index.ts (1)
30-30: Refactor the assignment in the expression.The static analysis tool flagged this line for having an assignment within an expression. This can make the code harder to understand.
- labelMap && (data = data.map((v) => (type.isNull(labelMap[v]) ? v : labelMap[v]))) + if (labelMap) { + data = data.map((v) => (type.isNull(labelMap[v]) ? v : labelMap[v])) + }🧰 Tools
🪛 Biome (1.9.4)
[error] 30-30: 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/common/validate/schema.ts (1)
349-349: Consider using optional chaining.The code can be simplified using optional chaining for better readability.
Apply this diff to improve the code:
-if (type.isFunction(rule.validator)) { +if (rule?.validator && type.isFunction(rule.validator)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 349-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/amount/index.ts (1)
327-331: Consider breaking down complex ternary expressions.While the change to use
type.isNumberis correct, the nested ternary expressions reduce readability.Consider refactoring to improve readability:
- fraction = type.isNumber(format.fraction) - ? format.fraction - : type.isNumber(serFra.fraction) - ? serFra.fraction - : digits + fraction = digits + if (type.isNumber(serFra.fraction)) { + fraction = serFra.fraction + } + if (type.isNumber(format.fraction)) { + fraction = format.fraction + }packages/renderless/src/common/string.ts (3)
Line range hint
224-242: Consider using template literals for string concatenation.The string concatenation in the
fillCharfunction could be simplified using template literals for better readability.- return append ? string + s : s + string + return append ? `${string}${s}` : `${s}${string}`
692-694: Consider consolidating the type checks.The sequential type checks could be combined for better efficiency.
- if (type.isNumber(value)) { - return value ? 1 : 0 - } else if (type.isNull(value) || value === 'false') { + if (type.isNull(value) || value === 'false') { return false + } else if (type.isNumber(value)) { + return value ? 1 : 0 }
781-781: Consider adding a descriptive error message.When the size is not a number or is negative, it would be helpful to provide a descriptive error message.
- } else if (!type.isNumber(size) || size <= 0) { + } else if (!type.isNumber(size) || size <= 0) { + console.warn('File size must be a positive number') return size + baseUnit }packages/renderless/src/popeditor/index.ts (2)
98-102: Consider simplifying the nested conditional.The nested conditional could be simplified using a ternary operator for better readability.
- if (type.isNull(state.selectedDatas)) { - return '' - } else { - return type.typeOf(state.selectedDatas) === 'object' ? state.selectedDatas[props.textField] : state.selectedDatas - } + return type.isNull(state.selectedDatas) ? '' : + (type.typeOf(state.selectedDatas) === 'object' ? state.selectedDatas[props.textField] : state.selectedDatas)🧰 Tools
🪛 eslint
[error] 101-101: Replace
·?·state.selectedDatas[props.textField]with⏎··········?·state.selectedDatas[props.textField]⏎·········(prettier/prettier)
312-312: Consider adding explicit type conversion.The type conversion for numeric values could be more explicit.
- value = type.typeOf(value) === 'number' ? (isNaN(value) ? '' : String(value)) : value + value = type.typeOf(value) === 'number' ? (Number.isNaN(value) ? '' : value.toString()) : value
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
.husky/pre-commit(1 hunks)examples/sites/demos/pc/webdoc/faq-en.md(1 hunks)examples/sites/demos/pc/webdoc/faq.md(1 hunks)packages/renderless/src/amount/index.ts(1 hunks)packages/renderless/src/base-select/index.ts(4 hunks)packages/renderless/src/base-select/vue.ts(2 hunks)packages/renderless/src/card/index.ts(2 hunks)packages/renderless/src/cascader-panel/index.ts(2 hunks)packages/renderless/src/cascader-panel/node.ts(2 hunks)packages/renderless/src/cascader/index.ts(4 hunks)packages/renderless/src/chart-bar/index.ts(5 hunks)packages/renderless/src/chart-candle/index.ts(4 hunks)packages/renderless/src/chart-core/deps/utils.ts(3 hunks)packages/renderless/src/chart-funnel/index.ts(2 hunks)packages/renderless/src/chart-gauge/index.ts(2 hunks)packages/renderless/src/chart-line/index.ts(3 hunks)packages/renderless/src/chart-map/index.ts(3 hunks)packages/renderless/src/chart-pie/index.ts(2 hunks)packages/renderless/src/chart-radar/index.ts(3 hunks)packages/renderless/src/chart-sankey/index.ts(2 hunks)packages/renderless/src/chart-scatter/index.ts(1 hunks)packages/renderless/src/chart-waterfall/index.ts(2 hunks)packages/renderless/src/checkbox/index.ts(4 hunks)packages/renderless/src/column-list-item/index.ts(2 hunks)packages/renderless/src/common/array.ts(3 hunks)packages/renderless/src/common/dataset/index.ts(3 hunks)packages/renderless/src/common/date.ts(7 hunks)packages/renderless/src/common/deps/date-util.ts(3 hunks)packages/renderless/src/common/deps/date.ts(4 hunks)packages/renderless/src/common/deps/dom.ts(5 hunks)packages/renderless/src/common/deps/popper.ts(3 hunks)packages/renderless/src/common/deps/tree-model/node.ts(3 hunks)packages/renderless/src/common/deps/tree-model/tree-store.ts(4 hunks)packages/renderless/src/common/deps/upload-ajax.ts(2 hunks)packages/renderless/src/common/function.ts(1 hunks)packages/renderless/src/common/object.ts(14 hunks)packages/renderless/src/common/runtime.ts(0 hunks)packages/renderless/src/common/string.ts(15 hunks)packages/renderless/src/common/validate/rules/range.ts(2 hunks)packages/renderless/src/common/validate/rules/required.ts(1 hunks)packages/renderless/src/common/validate/rules/type.ts(2 hunks)packages/renderless/src/common/validate/schema.ts(6 hunks)packages/renderless/src/common/validate/util.ts(3 hunks)packages/renderless/src/common/validate/validations/array.ts(1 hunks)packages/renderless/src/common/validate/validations/date.ts(1 hunks)packages/renderless/src/common/validate/validations/enum.ts(1 hunks)packages/renderless/src/common/validate/validations/float.ts(1 hunks)packages/renderless/src/common/validate/validations/integer.ts(1 hunks)packages/renderless/src/common/validate/validations/method.ts(1 hunks)packages/renderless/src/common/validate/validations/number.ts(1 hunks)packages/renderless/src/common/validate/validations/pattern.ts(1 hunks)packages/renderless/src/common/validate/validations/string.ts(1 hunks)packages/renderless/src/common/validate/validations/type.ts(1 hunks)packages/renderless/src/container/index.ts(2 hunks)packages/renderless/src/file-upload/index.ts(6 hunks)packages/renderless/src/file-upload/vue.ts(3 hunks)packages/renderless/src/fluent-editor/index.ts(2 hunks)packages/renderless/src/fluent-editor/options.ts(3 hunks)packages/renderless/src/form-item/index.ts(2 hunks)packages/renderless/src/grid/static/base/helperEqualCompare.ts(2 hunks)packages/renderless/src/grid/static/base/isPlainObject.ts(1 hunks)packages/renderless/src/grid/utils/common.ts(2 hunks)packages/renderless/src/image/index.ts(2 hunks)packages/renderless/src/modal/index.ts(2 hunks)packages/renderless/src/nav-menu/index.ts(3 hunks)packages/renderless/src/numeric/index.ts(5 hunks)packages/renderless/src/picker/index.ts(6 hunks)packages/renderless/src/popeditor/index.ts(4 hunks)packages/renderless/src/popup/index.ts(1 hunks)packages/renderless/src/recycle-scroller/index.ts(3 hunks)packages/renderless/src/rich-text/table-module.ts(8 hunks)packages/renderless/src/search/index.ts(3 hunks)packages/renderless/src/select/index.ts(4 hunks)packages/renderless/src/select/vue.ts(2 hunks)packages/renderless/src/skeleton/index.ts(1 hunks)packages/renderless/src/statistic/index.ts(2 hunks)packages/renderless/src/tabbar-item/index.ts(1 hunks)packages/renderless/src/toggle-menu/index.ts(2 hunks)packages/renderless/src/wizard/index.ts(2 hunks)packages/utils/src/type/index.ts(2 hunks)packages/vue-locale/package.json(1 hunks)packages/vue-locale/src/glob.ts(4 hunks)
⛔ Files not processed due to max files limit (11)
- packages/vue/src/grid-toolbar/package.json
- packages/vue/src/grid-toolbar/src/index.ts
- packages/vue/src/grid/src/body/src/body.tsx
- packages/vue/src/grid/src/footer/src/footer.ts
- packages/vue/src/grid/src/header/src/header.ts
- packages/vue/src/grid/src/table/src/methods.ts
- packages/vue/src/grid/src/table/src/table.ts
- packages/vue/src/grid/src/tools/formatter.ts
- packages/vue/src/image-viewer/src/mobileTouch.ts
- packages/vue/src/recycle-scroller/package.json
- packages/vue/src/recycle-scroller/src/idState.ts
💤 Files with no reviewable changes (1)
- packages/renderless/src/common/runtime.ts
✅ Files skipped from review due to trivial changes (2)
- .husky/pre-commit
- packages/renderless/src/grid/static/base/isPlainObject.ts
👮 Files not reviewed due to content moderation or server errors (8)
- packages/renderless/src/fluent-editor/index.ts
- packages/renderless/src/common/deps/popper.ts
- packages/renderless/src/picker/index.ts
- packages/renderless/src/base-select/index.ts
- packages/renderless/src/select/index.ts
- packages/renderless/src/file-upload/index.ts
- packages/vue-locale/package.json
- packages/renderless/src/statistic/index.ts
🧰 Additional context used
🪛 eslint
packages/renderless/src/picker/index.ts
[error] 238-238: Replace ·?·timezoneOffset·:·type.isNumber(TimezoneOffset)·?·TimezoneOffset with ⏎······?·timezoneOffset⏎······:·type.isNumber(TimezoneOffset)⏎········?·TimezoneOffset⏎·······
(prettier/prettier)
packages/renderless/src/popeditor/index.ts
[error] 101-101: Replace ·?·state.selectedDatas[props.textField] with ⏎··········?·state.selectedDatas[props.textField]⏎·········
(prettier/prettier)
packages/renderless/src/common/object.ts
[error] 232-232: Unexpected assignment within an 'if' statement.
(no-cond-assign)
packages/renderless/src/fluent-editor/options.ts
[error] 135-135: A constructor name should not start with a lowercase letter.
(new-cap)
🪛 Biome (1.9.4)
packages/renderless/src/common/validate/schema.ts
[error] 349-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/common/deps/date.ts
[error] 233-233: 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/chart-bar/index.ts
[error] 137-137: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 178-180: 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/chart-candle/index.ts
[error] 30-30: 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/cascader/index.ts
[error] 132-132: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (107)
packages/renderless/src/skeleton/index.ts (1)
1-1: LGTM! Clean refactoring of type utilities.The changes correctly migrate from local type utilities to the centralized @opentiny/utils package while maintaining the same functionality.
Also applies to: 4-4, 8-8
packages/renderless/src/common/function.ts (1)
1-1: LGTM! Consistent refactoring of promise type check.The migration to @opentiny/utils for promise type checking maintains the same functionality while aligning with the centralized type utilities approach.
Also applies to: 9-9
packages/renderless/src/common/validate/validations/method.ts (1)
15-15: LGTM! Consistent refactoring of property check.The migration to @opentiny/utils for property ownership checking maintains the same validation logic while aligning with the centralized type utilities approach.
Also applies to: 18-18
packages/renderless/src/common/validate/validations/enum.ts (1)
15-15: LGTM! Verify behavior consistency after utility migration.The migration from local type utility to @opentiny/utils aligns with the PR objectives. The validation logic remains intact.
Let's verify that the behavior of hasOwn remains consistent:
Also applies to: 21-21
✅ Verification successful
Behavior consistency verified after utility migration
The migration of
typeutility to@opentiny/utilsinenum.tsis correct and consistent with the codebase patterns. The remaining local type imports in index files are unrelated as they import local validator implementations rather than utility functions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any remaining local type utility imports rg -g '!node_modules' -l "from '.*type'" packages/renderless/src/common/validate/ # Check for consistent usage pattern across all files rg -g '!node_modules' "type\.hasOwn\.call" packages/renderless/src/common/validate/Length of output: 2543
Script:
#!/bin/bash # Check content of files still using local type imports echo "=== validations/index.ts ===" cat packages/renderless/src/common/validate/validations/index.ts echo -e "\n=== rules/index.ts ===" cat packages/renderless/src/common/validate/rules/index.tsLength of output: 2269
packages/renderless/src/common/validate/validations/pattern.ts (1)
15-15: LGTM! Consistent with the utility migration pattern.The changes follow the same pattern of centralizing type utilities while preserving the validation logic.
Also applies to: 19-19
packages/renderless/src/common/validate/validations/float.ts (1)
15-15: LGTM! Maintains validation integrity while standardizing utilities.The changes successfully migrate to the centralized type utility while preserving the float-specific validation logic.
Also applies to: 19-19
packages/renderless/src/common/validate/validations/array.ts (1)
15-15: LGTM! Verify @opentiny/utils version compatibility.The refactoring correctly moves from local type utility to the centralized @opentiny/utils package. The functionality remains unchanged.
Also applies to: 19-19
✅ Verification successful
Import change to @opentiny/utils is correctly configured
The change from local type utility to @opentiny/utils is safe as:
- @opentiny/utils exists as a workspace package
- It's properly listed as a dependency in packages/renderless
- The type utility exports are available in the centralized package
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify @opentiny/utils package version and its exports if [ -f "package.json" ]; then # Check the version specified in package.json jq -r '.dependencies["@opentiny/utils"] // .devDependencies["@opentiny/utils"]' package.json # Verify the type export is available in the specified version rg -p "export.*type" $(fd -t f "index.(js|ts)" -d 3 node_modules/@opentiny/utils) fiLength of output: 65990
Script:
#!/bin/bash # Check if @opentiny/utils is available in the workspace fd -t f package.json | xargs cat | jq -r 'select(.dependencies["@opentiny/utils"] or .devDependencies["@opentiny/utils"]) | "\(.name): \(.dependencies["@opentiny/utils"] // .devDependencies["@opentiny/utils"])"' # Check if @opentiny/utils is a workspace package fd -t f -p '@opentiny/utils/package.json$'Length of output: 353
Script:
#!/bin/bash # Check root package.json if [ -f "package.json" ]; then echo "Root package.json dependencies:" grep -A 5 '"@opentiny/utils"' package.json || echo "Not found in root" fi # Check renderless package.json if [ -f "packages/renderless/package.json" ]; then echo -e "\nRenderless package.json dependencies:" grep -A 5 '"@opentiny/utils"' packages/renderless/package.json || echo "Not found in renderless" fi # Check if utils is a workspace package if [ -f "packages/utils/package.json" ]; then echo -e "\nUtils package.json:" grep '"name"' packages/utils/package.json || echo "Name not found" fiLength of output: 670
packages/renderless/src/common/validate/validations/type.ts (1)
15-15: LGTM! Consistent with the refactoring pattern.The changes maintain consistency with other files by using the centralized type utilities from @opentiny/utils.
Also applies to: 20-20
packages/renderless/src/common/validate/validations/number.ts (1)
15-15: LGTM! Maintains existing validation logic.The refactoring correctly preserves the special handling of empty string values while moving to centralized type utilities.
Also applies to: 19-19
packages/renderless/src/common/validate/validations/date.ts (1)
15-15: LGTM! Verify other date validation files.The refactoring to use
@opentiny/utilsis correct. The validation logic remains unchanged.Let's verify if all date validation files have been updated consistently:
Also applies to: 19-19
✅ Verification successful
Verification complete: Date-related files are consistent
The refactoring to use
@opentiny/utilsis properly implemented across the codebase. Both the validation file and the main date utility file consistently use the centralized type utilities from@opentiny/utils.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all date validation files and check for any remaining local type imports rg -l "from '.*type'" packages/renderless/src/**/date*.tsLength of output: 68
Script:
#!/bin/bash # First find all date-related files echo "=== Date-related files ===" fd -t f "date" packages/renderless/src # Then check imports in these files echo -e "\n=== Type imports in date files ===" rg "import.*type" packages/renderless/src/**/date*.tsLength of output: 527
Script:
#!/bin/bash # Check for type-related utility usage in date files echo "=== Checking type usage in date files ===" rg "type\." packages/renderless/src/**/date*.ts -A 2 # Also check if files import from each other echo -e "\n=== Checking imports between date files ===" rg "import.*date" packages/renderless/src/**/date*.tsLength of output: 998
Script:
#!/bin/bash # Check content of the validation file echo "=== Validation file content ===" cat packages/renderless/src/common/validate/validations/date.tsLength of output: 1673
packages/renderless/src/tabbar-item/index.ts (1)
13-13: LGTM! Type checks properly refactored.The migration to
@opentiny/utilstype utilities maintains the same logical checks while standardizing the type-checking approach.Also applies to: 19-19, 21-21
packages/renderless/src/common/deps/upload-ajax.ts (1)
13-13: LGTM! Verify security implications of header handling.The refactoring to use
type.hasOwnis correct. The header validation remains secure with proper null checks.Let's verify security measures across upload handlers:
Also applies to: 105-105
✅ Verification successful
Security measures verified across upload handlers
All upload implementations consistently apply URL sanitization via
xss.filterUrland follow secure header handling patterns. The changes to usetype.hasOwnmaintain the existing security model.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent security patterns in upload handlers rg -A 3 "setRequestHeader|xss\.filter" packages/renderless/src/**/upload*.tsLength of output: 158
Script:
#!/bin/bash # First find upload-related files fd -e ts upload packages/renderless/src # Then search for security-related patterns echo "=== Security patterns ===" rg -A 3 "setRequestHeader|xss\.filter|sanitize|escape" $(fd -e ts upload packages/renderless/src)Length of output: 2581
packages/renderless/src/column-list-item/index.ts (1)
1-1: LGTM! Import and type check refactored correctly.The change correctly moves the type checking functionality to the @opentiny/utils package while maintaining the same behavior.
Also applies to: 51-51
packages/renderless/src/card/index.ts (1)
1-1: LGTM! Consistent with the refactoring pattern.The changes align with the codebase-wide refactoring effort to use @opentiny/utils for type checking.
Also applies to: 76-76
packages/renderless/src/grid/static/base/helperEqualCompare.ts (1)
26-26: LGTM! Type checks refactored correctly.The changes correctly move RegExp and Date type checking to @opentiny/utils while maintaining the same behavior.
Also applies to: 42-42, 46-46
packages/renderless/src/toggle-menu/index.ts (1)
13-13: LGTM! Import consolidated and type check refactored correctly.The changes efficiently combine imports from @opentiny/utils and correctly refactor the type checking functionality.
Also applies to: 51-51
packages/renderless/src/popup/index.ts (1)
16-16: LGTM! Import and usage of type utility correctly updated.The migration from local type utility to
@opentiny/utilsis implemented correctly, maintaining the same functionality while improving code organization.Also applies to: 21-21
packages/renderless/src/chart-sankey/index.ts (1)
15-15: LGTM! Import of type utility correctly updated.The migration from local type utility to
@opentiny/utilsis implemented correctly.packages/renderless/src/chart-gauge/index.ts (1)
15-15: LGTM! Import and usage of type utility correctly updated.The migration from local type utility to
@opentiny/utilsis implemented correctly, maintaining the same functionality while improving code organization.Also applies to: 61-61
packages/renderless/src/container/index.ts (1)
13-13: LGTM! Import of type utility correctly updated.The migration from local type utility to
@opentiny/utilsis implemented correctly.packages/renderless/src/chart-radar/index.ts (1)
15-15: LGTM! Verify the impact of utility function changes.The changes to use
type.isNullfrom@opentiny/utilsare consistent with the PR's objective of centralizing utility functions. The functionality remains unchanged.Let's verify that all
isNullimports from the local type module have been replaced:Also applies to: 21-21, 66-66, 102-102
✅ Verification successful
Utility function changes verified successfully
The changes to use
type.isNullfrom@opentiny/utilsin the reviewed file are correct. The remaining local imports ofisNullare confined to the self-contained huicharts module and don't affect the overall migration objective.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining local imports of isNull rg "import.*\{.*isNull.*\}.*from.*['\"].*common/type['\"]" # Search for any remaining direct usage of isNull without type prefix rg "\bisNull\(" --type tsLength of output: 16491
packages/renderless/src/common/array.ts (2)
126-126: LGTM! Improved type comparison using utility function.The change to use
type.isSamefrom@opentiny/utilsaligns with the codebase's move towards centralized type utilities.
203-206: Verify the impact of switching to strict equality.The change from loose equality (
==) to strict equality (===) for pid comparison might affect existing data where pid types don't exactly match (e.g., string "0" vs number 0).Run this script to check for potential type mismatches in existing usage:
✅ Verification successful
The strict equality change is correct and safe
The codebase consistently treats pid as string type, with "0" being the expected value for root nodes. The strict equality check aligns with the type system and prevents potential type coercion issues. For consistency, the mobile version in
packages/mobile/utils/array.tsshould also be updated to use strict equality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential pid type mismatches that might be affected by strict equality # Look for pid values in the codebase to identify potential type inconsistencies rg -A 2 'pId["\']?\s*[:=]\s*(["\'0-9]+)' --type ts --type jsLength of output: 243
Script:
#!/bin/bash # Search for pid/parentId patterns in variable declarations and assignments rg -i "pid|parentid" --type ts # Search for type definitions involving pid/parentId ast-grep --pattern 'interface $_ { $$$ pid: $_ $$$ }' # Search for actual usage in function calls or comparisons rg "(pid|parentId)\s*(===?|==)\s*['\"]?0['\"]?" --type tsLength of output: 12047
packages/renderless/src/grid/utils/common.ts (1)
87-87: LGTM! Consistent null checking using utility function.The change to use
type.isNullfrom@opentiny/utilsmaintains consistency with the codebase's type checking approach.packages/renderless/src/chart-map/index.ts (2)
28-28: LGTM! Consistent null checking in formatter function.The change to use
type.isNullfrom@opentiny/utilsaligns with the standardized approach to type checking.
60-60: LGTM! Consistent null checking in getSeries function.The change to use
type.isNullfrom@opentiny/utilsmaintains consistency in type checking implementation.packages/renderless/src/chart-core/deps/utils.ts (4)
14-15: LGTM! Import changes align with centralization objective.The change to import
typefrom@opentiny/utilsaligns with the PR's objective of centralizing type-related utilities.
Line range hint
27-33: LGTM! Consistent usage of centralized type checking.The change to use
type.isObjectmaintains the same functionality while leveraging the centralized type utilities.
45-46: LGTM! Consistent type checking implementation.The change to use
type.typeOfmaintains the same functionality while using the centralized type utilities.
205-211: LGTM! Consistent null checking implementation.The changes to use
type.isNullmaintain the same functionality while using the centralized type utilities.packages/renderless/src/image/index.ts (2)
15-15: LGTM! Import changes align with centralization objective.The change to import
typefrom@opentiny/utilsaligns with the PR's objective of centralizing type-related utilities.
109-109: LGTM! Consistent type checking implementation.The change to use
type.typeOfmaintains the same functionality while using the centralized type utilities.packages/renderless/src/common/validate/util.ts (3)
13-13: LGTM! Import changes align with centralization objective.The change to import
typefrom@opentiny/utilsaligns with the PR's objective of centralizing type-related utilities.
112-112: LGTM! Consistent null checking implementation.The change to use
type.isNullmaintains the same functionality while using the centralized type utilities.
276-276: LGTM! Consistent property ownership checking.The change to use
type.hasOwnmaintains the same functionality while using the centralized type utilities.packages/renderless/src/chart-pie/index.ts (2)
15-15: LGTM! Import changes align with centralization objective.The change to import
typefrom@opentiny/utilsaligns with the PR's objective of centralizing type-related utilities.
126-126: LGTM! Consistent null checking implementation.The change to use
type.isNullmaintains the same functionality while using the centralized type utilities.packages/renderless/src/checkbox/index.ts (3)
14-14: LGTM!The import of
typeutility from@opentiny/utilsis correctly added.
75-75: LGTM!The null check is correctly updated to use
type.isNullfrom the utils package.
107-107: LGTM!The null check in the condition is correctly updated to use
type.isNull.packages/renderless/src/chart-scatter/index.ts (2)
16-16: LGTM!The import of
typeutility from@opentiny/utilsis correctly added.
20-20: LGTM!The null check in the formatter function is correctly updated to use
type.isNull.packages/renderless/src/chart-line/index.ts (2)
14-15: LGTM!The import of
typeutility from@opentiny/utilsis correctly added.
67-67: LGTM!The null checks in the
getLineSeriesfunction are correctly updated to usetype.isNull.Also applies to: 78-78
packages/renderless/src/chart-candle/index.ts (2)
15-15: LGTM!The import of
typeutility from@opentiny/utilsis correctly added.
33-33: LGTM!The null checks across all functions are correctly updated to use
type.isNull.Also applies to: 59-59, 67-67, 165-165, 171-171
packages/renderless/src/common/deps/dom.ts (3)
13-14: LGTM!The imports from
@opentiny/utilsalign with the PR objective of centralizing type utilities.
129-129: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities.
198-198: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities.packages/renderless/src/common/deps/date.ts (3)
14-14: LGTM!The import from
@opentiny/utilsaligns with the PR objective of centralizing type utilities.
276-276: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities.
333-333: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities.packages/renderless/src/common/deps/date-util.ts (3)
16-16: LGTM!The import from
@opentiny/utilsaligns with the PR objective of centralizing type utilities.
42-42: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities.
213-213: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities.packages/renderless/src/common/validate/schema.ts (5)
14-14: LGTM!The import from
@opentiny/utilsaligns with the PR objective of centralizing type utilities.
85-85: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities.
97-97: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities.
228-228: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities.
371-371: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities.packages/renderless/src/common/deps/tree-model/tree-store.ts (3)
15-15: LGTM! Import statement aligns with PR objectives.The change to import
typefrom@opentiny/utilsaligns with the PR's goal of centralizing type utilities.
23-23: LGTM! Consistent usage of type.hasOwn.The replacement of local
hasOwnwithtype.hasOwnis consistent across all instances in the file.Also applies to: 48-48, 251-251
381-381: LGTM! Consistent usage of type.isNull.The replacement of local
isNullwithtype.isNullis consistent with the PR's refactoring goals.packages/renderless/src/amount/index.ts (1)
336-336: LGTM! Consistent usage of type.isNumber.The replacement of local
isNumberwithtype.isNumberis consistent with the PR's refactoring goals.packages/renderless/src/common/object.ts (1)
13-13: LGTM! Consistent usage of type utilities.The replacement of local type utility functions with centralized imports from
@opentiny/utilsis consistent throughout the file and aligns with the PR's objectives.Also applies to: 33-33, 64-64, 79-79, 107-107, 127-127, 138-138, 144-144, 197-197, 255-255, 261-261, 268-268, 321-321, 342-342, 366-366, 370-370, 424-424
packages/renderless/src/cascader-panel/index.ts (1)
13-13: LGTM! Verify the import usage.The change from local
isNulltotype.isNullmaintains the same functionality while aligning with the centralized type-checking utilities.Run the following script to verify consistent usage across the codebase:
Also applies to: 93-93
✅ Verification successful
Import change verified successfully
The codebase shows consistent usage of
type.isNullfrom '@opentiny/utils' with no remaining direct imports ofisNull.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of type utilities # Test: Check if there are any remaining direct imports of isNull rg "import.*\{.*isNull.*\}.*from.*['\"]\.\./common/type['\"]" # Test: Verify the new pattern is used consistently rg "type\.isNull" -A 2Length of output: 20944
packages/renderless/src/file-upload/vue.ts (1)
101-101: LGTM! Consistent use of centralized utilities.The change from local
isEmptyObjecttotype.isEmptyObjectin computed properties maintains functionality while improving code consistency.Also applies to: 180-180, 197-197
packages/renderless/src/rich-text/table-module.ts (1)
2-2: LGTM! Consistent null checks across table operations.The changes from local
isNulltotype.isNullmaintain the same functionality while standardizing null checks throughout the table module.Also applies to: 93-93, 223-223, 261-261, 341-341, 373-373, 430-430, 445-445
packages/renderless/src/common/deps/tree-model/node.ts (2)
16-16: LGTM! Consistent type checking in property access.The changes from local
typeOftotype.typeOfmaintain the same functionality while standardizing type checking.Also applies to: 26-28
164-164: LGTM! Consistent property ownership check.The change from local
hasOwntotype.hasOwnmaintains the same functionality while standardizing property access checks.packages/renderless/src/common/date.ts (7)
13-13: LGTM! Import statement updated to use centralized type utility.The change aligns with the PR objective of centralizing type utilities.
363-363: LGTM! Type check updated intoDatefunction.The change correctly uses the centralized type utility for number type checking.
407-407: LGTM! Type check updated informatfunction.The change correctly uses the centralized type utility for date type checking.
458-458: LGTM! Multiple type checks updated ingetDateWithNewTimezone.The change correctly uses the centralized type utility for date and numeric type checking.
482-485: LGTM! Type checks updated intoDateStr.The change correctly uses the centralized type utility for numeric and date type checking.
505-505: LGTM! Type check updated ingetWeekOfFirstDay.The change correctly uses the centralized type utility for date type checking.
532-532: LGTM! Type check updated ingetStrTimezone.The change correctly uses the centralized type utility for number type checking.
packages/renderless/src/form-item/index.ts (2)
17-17: LGTM! Import statement updated to use centralized type utility.The change aligns with the PR objective of centralizing type utilities.
33-33: LGTM! Type check updated inwatchErrorfunction.The change correctly uses the centralized type utility for null checking.
packages/renderless/src/numeric/index.ts (5)
24-25: LGTM! Import statement updated to use centralized type utility.The change aligns with the PR objective of centralizing type utilities.
204-204: LGTM! Type check updated inhandleFocusfunction.The change correctly uses the centralized type utility for null checking.
391-396: LGTM! Type checks updated inmountedfunction.The change correctly uses the centralized type utility for number type checking.
358-358: LGTM! Type check updated inhandleInputfunction.The change correctly uses the centralized type utility for null checking.
499-500: LGTM! Type checks updated ingetUnitPrecisionfunction.The change correctly uses the centralized type utility for number type checking.
packages/renderless/src/recycle-scroller/index.ts (3)
2-2: LGTM! Import statement updated to use centralized type utility.The change aligns with the PR objective of centralizing type utilities.
491-491: LGTM! Type check updated incomputePoolfunction.The change correctly uses the centralized type utility for null checking.
543-543: LGTM! Type check updated inunuseInvisiblefunction.The change correctly uses the centralized type utility for null checking.
packages/renderless/src/base-select/vue.ts (2)
103-103: LGTM! Import change aligns with refactoring goal.The change to import
typefrom@opentiny/utilsaligns with the PR objective of centralizing type utilities.
268-268: LGTM! Consistent use of centralized type utility.The change from local
isNumbertotype.isNumbermaintains the same functionality while using the centralized utility.packages/renderless/src/nav-menu/index.ts (3)
21-21: LGTM! Import change aligns with refactoring goal.The change to import
typefrom@opentiny/utilsaligns with the PR objective of centralizing type utilities.
44-47: LGTM! Consistent use of centralized type utility.The change from local
isEmptyObjecttotype.isEmptyObjectmaintains the same functionality while using the centralized utility.
185-185: LGTM! Consistent use of centralized type utility.The change from local
isObjecttotype.isObjectmaintains the same functionality while using the centralized utility.packages/renderless/src/cascader/index.ts (4)
14-14: LGTM! Import change aligns with refactoring goal.The change to import
typefrom@opentiny/utilsaligns with the PR objective of centralizing type utilities.
51-51: LGTM! Consistent use of centralized type utility.The change from local
isNulltotype.isNullmaintains the same functionality while using the centralized utility.
229-229: LGTM! Consistent use of centralized type utility.The change from local
isNulltotype.isNullmaintains the same functionality while using the centralized utility.
132-132:⚠️ Potential issueFix variable declaration order.
The
typevariable is used before its declaration in theisEmptyfunction. This could lead to runtime errors.Move the variable declaration before its usage:
- const type = Object.prototype.toString.call(val) + const objType = Object.prototype.toString.call(val) - if (~['[object String]', '[object Array]'].indexOf(type)) { + if (~['[object String]', '[object Array]'].indexOf(objType)) { return !val.length } - if (~['[object File]', '[object Map]', '[object Set]'].indexOf(type)) { + if (~['[object File]', '[object Map]', '[object Set]'].indexOf(objType)) { return !val.size } - if (type === '[object Object]') { + if (objType === '[object Object]') { return !Object.keys(val).length }Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 132-132: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
packages/renderless/src/select/vue.ts (2)
116-116: LGTM! Import change aligns with refactoring goal.The change to import
typefrom@opentiny/utilsaligns with the PR objective of centralizing type utilities.
326-326: LGTM! Consistent use of centralized type utility.The change from local
isNumbertotype.isNumbermaintains the same functionality while using the centralized utility.packages/renderless/src/common/string.ts (2)
Line range hint
652-656: LGTM! Numeric type check is properly implemented.The type check using
type.isNumericis correctly implemented, ensuring proper validation before currency conversion.🧰 Tools
🪛 Biome (1.9.4)
[error] 651-651: 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)
717-719: LGTM! Rate calculation is properly guarded.The type checks ensure that both value and total are numbers before performing rate calculation.
packages/renderless/src/modal/index.ts (1)
Line range hint
282-289: LGTM! Viewport window access is properly updated.The viewport window access is correctly updated to use
global.getViewportWindow(), maintaining consistent access across the codebase.packages/renderless/src/fluent-editor/options.ts (1)
105-144: LGTM! List autofill handler is properly implemented.The list autofill handler correctly uses
type.isNullfor checking the list query result, ensuring proper validation before proceeding with the autofill operation.🧰 Tools
🪛 eslint
[error] 135-135: A constructor name should not start with a lowercase letter.
(new-cap)
packages/renderless/src/popeditor/index.ts (1)
Line range hint
174-182: LGTM! Commit value validation is properly implemented.The type check using
type.isNullcorrectly validates the commit value before proceeding with the display update and emit operations.examples/sites/demos/pc/webdoc/faq.md (1)
11-11: LGTM! Import and configuration access updated correctly.The changes align with the PR objective of centralizing utilities in the
@opentiny/utilspackage. The global configuration access pattern has been updated consistently.Also applies to: 16-16
examples/sites/demos/pc/webdoc/faq-en.md (1)
11-11: LGTM! Changes are consistent with Chinese version.The import and configuration updates mirror those in the Chinese documentation, maintaining consistency across language versions.
Also applies to: 16-16
packages/renderless/src/chart-funnel/index.ts (2)
15-15: LGTM! Import updated to use centralized utilities.The change aligns with the PR objective of moving type utilities to the centralized
@opentiny/utilspackage.
34-34: LGTM! Type check updated correctly.The
isNullcheck has been properly updated to use the centralized utility viatype.isNull.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
packages/renderless/src/popup/index.ts (1)
Documentation update needed for type utilities migration
The verification confirms that documentation updates are needed. No existing documentation was found covering the migration to centralized type utilities from
@opentiny/utils. This documentation should be added to help users understand and implement the changes.🔗 Analysis chain
Line range hint
1-1: Documentation update needed.According to the PR objectives, documentation updates for the changes have not been included. Consider adding documentation to explain the migration to centralized type utilities from
@opentiny/utils.Let's check for existing documentation:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for documentation files that might need updates fd -e md -e mdx | grep -i "doc\|guide"Length of output: 46524
Script:
#!/bin/bash # Search for relevant documentation content echo "=== Searching for type utilities documentation ===" rg -i "type.*util" examples/sites/demos/pc/webdoc/changelog.md examples/sites/demos/pc/webdoc/migration || true echo -e "\n=== Searching for popup component documentation ===" rg -i "popup" examples/sites/demos/pc/webdoc/changelog.md examples/sites/demos/pc/webdoc/migration || true echo -e "\n=== Checking if popup component has dedicated docs ===" fd popup examples/sites/demos/pc/app -t d || trueLength of output: 1757
Script:
#!/bin/bash # Search in mobile docs and root level documentation echo "=== Searching mobile docs for type utilities and popup ===" rg -i "type.*util|migration|popup" examples/sites/demos/mobile/webdoc/ || true echo -e "\n=== Checking root level documentation ===" rg -i "type.*util|migration|upgrade" examples/docs/README.md || true echo -e "\n=== Checking for popup in mobile demos ===" fd popup examples/sites/demos/mobile/app -t d || trueLength of output: 524
packages/renderless/src/common/array.ts (1)
Line range hint
29-29: Fix inconsistent default parameter.The
indexOffunction still references the oldisSamefunction as its default parameter, which should be updated totype.isSamefor consistency.-export const indexOf = (arr, data, predicate = isSame) => { +export const indexOf = (arr, data, predicate = type.isSame) => {
🧹 Nitpick comments (27)
packages/renderless/src/common/validate/validations/type.ts (1)
20-20: LGTM! Consider adding documentation.The change to
type.hasOwn.call()maintains the same functionality while using the centralized utility. However, since documentation updates weren't included in the PR, consider adding:
- JSDoc comments explaining the validation logic
- Updates to any relevant API documentation
packages/renderless/src/grid/static/base/isPlainObject.ts (1)
13-13: LGTM! Consider adding JSDoc for the re-exported function.The refactoring correctly exports the utility from @opentiny/utils. Consider adding a JSDoc comment to document this utility's purpose for better maintainability.
import { type } from '@opentiny/utils' +/** + * Checks if value is a plain object. + * @param {*} value The value to check + * @returns {boolean} Returns true if value is a plain object, else false + */ export default type.isPlainObjectAlso applies to: 15-15
packages/renderless/src/common/function.ts (1)
1-1: LGTM! Consider adding TypeScript types for better type safety.The migration to @opentiny/utils is correct. Consider adding TypeScript types for the interceptor parameters to improve type safety and developer experience.
-export const callInterceptor = (interceptor, { args = [], done, canceled, error }) => { +type InterceptorCallback = () => void; +type InterceptorOptions = { + args?: any[]; + done: InterceptorCallback; + canceled?: InterceptorCallback; + error?: (error: Error) => void; +}; + +export const callInterceptor = ( + interceptor: (...args: any[]) => any, + { args = [], done, canceled, error }: InterceptorOptions +) => {Also applies to: 9-9
packages/renderless/src/common/validate/rules/required.ts (1)
14-14: Consider using a more descriptive parameter name than '_type'.While renaming the parameter to avoid conflict with the imported
typeis necessary, using '_type' is not very descriptive. Consider using a more meaningful name like 'valueType' or 'fieldType' to better indicate its purpose.-export default function ({ rule, checkValue, source, errors, options, _type }) { - if (rule.required && (!type.hasOwn.call(source, rule.field) || util.isEmptyValue(checkValue, type || rule._type))) { +export default function ({ rule, checkValue, source, errors, options, valueType }) { + if (rule.required && (!type.hasOwn.call(source, rule.field) || util.isEmptyValue(checkValue, type || valueType))) {Also applies to: 16-17
packages/renderless/src/common/validate/validations/float.ts (1)
15-15: Consider extracting common validation setup.While the change to use
type.hasOwnis correct, I notice this validation setup is repeated across multiple validation files (pattern.ts, float.ts, integer.ts).Consider extracting the common validation setup into a shared utility:
// common/validate/utils/validateSetup.ts export const setupValidation = (rule: Rule, source: Source) => { return rule.required || (!rule.required && type.hasOwn.call(source, rule.field)) }This would reduce duplication and make future updates easier.
Also applies to: 19-19
packages/renderless/src/common/validate/validations/integer.ts (1)
15-15: LGTM! Part of consistent validation module updates.The change to use
type.hasOwnfrom@opentiny/utilsis correct and consistent with other validation files.Given the identical validation setup across multiple files (pattern.ts, float.ts, integer.ts), consider:
- Creating a shared validation setup utility
- Potentially consolidating the validation rules into a more maintainable structure
- Adding unit tests to verify the behavior remains consistent after these utility migrations
Would you like me to propose a detailed refactoring plan for the validation module?
Also applies to: 19-19
packages/renderless/src/common/validate/validations/date.ts (1)
15-19: Consider documenting the type utility centralization.The centralization of type utilities from local implementations to
@opentiny/utilsis a good architectural decision that promotes consistency and maintainability. Consider:
- Updating the documentation to reflect this architectural change
- Adding migration guides for other teams who might be using the local type utilities
packages/renderless/src/common/validate/rules/range.ts (1)
14-14: LGTM! Consider consolidating the number type checks.The changes correctly use the centralized type utilities. However, there's an opportunity to improve the code further.
Consider consolidating the repeated number checks into a single variable:
- const len = type.isNumber(rule.len) - const min = type.isNumber(rule.min) - const max = type.isNumber(rule.max) + const { len, min, max } = ['len', 'min', 'max'].reduce((acc, key) => ({ + ...acc, + [key]: type.isNumber(rule[key]) + }), {})Also applies to: 28-30, 33-33
packages/renderless/src/common/deps/upload-ajax.ts (1)
13-13: LGTM! Consider additional header validation.The change correctly uses the centralized type utility for property checking. The XSS filtering is properly maintained through
xss.filterUrl.Consider adding validation for header values to prevent header injection:
if (type.hasOwn.call(headers, header) && headers[header] !== null) { + const headerValue = String(headers[header]).replace(/[\r\n]+/g, ''); - xhr.setRequestHeader(header, headers[header]) + xhr.setRequestHeader(header, headerValue) }Also applies to: 105-105
packages/renderless/src/column-list-item/index.ts (1)
1-1: LGTM! Consider enhancing type safety.The changes correctly use the centralized type utility. The code is now more consistent with the rest of the codebase.
Consider adding a type guard to improve type safety:
- return type.isNull(model) ? [] : model + return type.isNull(model) ? [] : Array.isArray(model) ? model : [model]Also applies to: 51-51
packages/renderless/src/container/index.ts (1)
13-13: LGTM! Type utility migration looks good.The migration to
@opentiny/utilsis implemented correctly. The number type check functionality intransferWidthOrHeightis preserved while maintaining the same behavior across all style computations.Consider enhancing dimension handling.
Consider creating a dedicated utility function for dimension handling that could include additional validations and unit conversions. This would make the code more maintainable and reusable across different components. Example:
-const transferWidthOrHeight = (value) => (type.isNumber(value) ? value + 'px' : value) +const toDimension = (value, unit = 'px') => { + if (type.isNumber(value)) { + return `${value}${unit}` + } + if (typeof value === 'string' && /^-?\d*\.?\d+$/.test(value)) { + return `${value}${unit}` + } + return value +}Also applies to: 30-30
packages/vue-locale/src/glob.ts (1)
11-11: Consider adding type assertions for critical type checks.The changes affect multiple critical type checks in formatting logic:
- Plain object validation in number formatting
- Number range validation
- Date object validation
- Null checks in date formatting
Consider adding TypeScript type assertions to strengthen type safety:
-if (type.isDate(value)) { +if (type.isDate(value)): value is Date {Also applies to: 52-52, 97-97, 101-101, 114-114
packages/renderless/src/wizard/index.ts (1)
16-16: Consider handling circular references in cloneDeep.The change to
type.isObjectin cloneDeep should handle circular references to prevent stack overflow:+const seen = new WeakSet(); if (type.isObject(data)) { + if (seen.has(data)) { + return data; + } + seen.add(data); return extend(true, data) }Also applies to: 79-79
packages/renderless/src/chart-radar/index.ts (1)
15-15: Consider adding TypeScript type annotations.The changes correctly replace the local type utility with
@opentiny/utils. However, to improve type safety, consider adding TypeScript type annotations for the parameters:-function formatter(name) { +function formatter(name: string) { return type.isNull(legendName[name]) ? name : legendName[name] } -const key = type.isNull(labelMap[item]) ? item : labelMap[item] +const key = type.isNull(labelMap[item as keyof typeof labelMap]) ? item : labelMap[item] -let k = type.isNull(labelMap[key]) ? radarIndexObj[key] : radarIndexObj[labelMap[key]] +let k = type.isNull(labelMap[key as keyof typeof labelMap]) ? radarIndexObj[key] : radarIndexObj[labelMap[key]]Also applies to: 21-21, 66-66, 102-102
packages/renderless/src/search/index.ts (1)
94-96: Extract duplicate validation logic into a helper function.The same validation logic is repeated in both
setDefaultTypeandformatSearchTypesfunctions. Consider extracting it into a reusable helper:+const isValidSearchType = (type: ISearchValue) => + type.isObject(type) && + type.typeOf(type.value) !== 'undefined' && + type.typeOf(type.text) !== 'undefined' export const setDefaultType = (searchTypes: ISearchValue[], typeValue: ISearchValue): ISearchValue => { // ... - if ( - type.isObject(searchTypes[i]) && - type.typeOf(searchTypes[i].value) !== 'undefined' && - type.typeOf(searchTypes[i].text) !== 'undefined' - ) { + if (isValidSearchType(searchTypes[i])) { type = searchTypes[i] break } // ... } export const formatSearchTypes = (searchTypes: ISearchValue[]): ISearchValue[] => { // ... - if ( - type.isObject(searchTypes[i]) && - type.typeOf(searchTypes[i].value) !== 'undefined' && - type.typeOf(searchTypes[i].text) !== 'undefined' - ) { + if (isValidSearchType(searchTypes[i])) { types.push(searchTypes[i]) } // ... }Also applies to: 111-113
packages/utils/src/type/index.ts (1)
166-166: Add JSDoc documentation for the isPromise function.Other utility functions in this file have comprehensive documentation with examples. Consider adding similar documentation for the
isPromisefunction:+/** + * Determines if a value is a Promise. + * + * @param {any} val - The value to test + * @returns {boolean} True if the value is a Promise + * + * @example + * isPromise(Promise.resolve()) // true + * isPromise(new Promise(() => {})) // true + * isPromise({ then: () => {}, catch: () => {} }) // true + * isPromise({}) // false + */ export const isPromise = (val: any) => isObject(val) && isFunction(val.then) && isFunction(val.catch)packages/renderless/src/chart-waterfall/index.ts (1)
52-52: Consider using nullish coalescing operator.The null check can be simplified using the nullish coalescing operator:
-let name = !type.isNull(labelMap[yAxisName]) ? labelMap[yAxisName] : yAxisName +let name = labelMap[yAxisName] ?? yAxisNamepackages/renderless/src/chart-map/index.ts (1)
15-15: LGTM! Consider adding type safety improvements.The migration to centralized type utilities is correct. However, consider adding TypeScript type annotations for the
labelMapparameter to improve type safety and IDE support.-const getTooltip = ({ dataType, digit, dataStore, metrics, color, labelMap }) => { +interface LabelMap { + [key: string]: string | null +} + +const getTooltip = ({ dataType, digit, dataStore, metrics, color, labelMap }: { labelMap: LabelMap }) => {Also applies to: 28-28, 60-60
packages/renderless/src/common/array.ts (1)
203-203: Consider using strict equality consistently.The change to use strict equality (
===) is good. For consistency, consider applying the same to other comparison operations in the file.Also applies to: 206-206
packages/renderless/src/checkbox/index.ts (1)
14-14: Overall assessment: Changes successfully centralize type utilities.The PR consistently replaces local type utilities with centralized ones from @opentiny/utils across all files, improving maintainability without introducing breaking changes.
Consider adding JSDoc comments to document the centralized type utilities in @opentiny/utils for better developer experience.
packages/renderless/src/chart-line/index.ts (1)
14-15: Consider separating the assignment from the condition.While the type utility migration looks good, there's a potential readability issue with combining assignment and condition:
- let name = !type.isNull(labelMap[item]) ? labelMap[item] : item + let name; + name = !type.isNull(labelMap[item]) ? labelMap[item] : itemThis change would improve code clarity without affecting functionality.
Also applies to: 67-67, 78-78
packages/renderless/src/common/validate/schema.ts (1)
349-349: Consider using optional chaining.The code can be simplified using optional chaining for better readability.
Apply this diff:
- if (type.isFunction(rule.validator)) { + if (rule?.validator && type.isFunction(rule.validator)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 349-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/chart-bar/index.ts (1)
178-180: Refactor assignment in expression for better readability.The assignment within the expression can be confusing. Consider refactoring for clarity:
-dimAxisType === 'value' && type.isNull(itemOpacity) && (itemOpacity = VALUE_AXIS_OPACITY) +if (dimAxisType === 'value' && type.isNull(itemOpacity)) { + itemOpacity = VALUE_AXIS_OPACITY +}🧰 Tools
🪛 Biome (1.9.4)
[error] 178-180: 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/common/object.ts (1)
232-232: Refactor assignment in if condition for better readability.The assignment within the if condition can be confusing. Consider refactoring for clarity:
-if (deep && copy && (type.isPlainObject(copy) || (copyIsArray = Array.isArray(copy)))) { +const isArray = Array.isArray(copy) +if (deep && copy && (type.isPlainObject(copy) || isArray)) { + copyIsArray = isArray🧰 Tools
🪛 eslint
[error] 232-232: Unexpected assignment within an 'if' statement.
(no-cond-assign)
packages/renderless/src/popeditor/index.ts (1)
98-102: Consider improving the type checking logic.The nested ternary operation could be simplified for better readability.
Consider this alternative:
- if (type.isNull(state.selectedDatas)) { - return '' - } else { - return type.typeOf(state.selectedDatas) === 'object' ? state.selectedDatas[props.textField] : state.selectedDatas - } + if (type.isNull(state.selectedDatas)) return '' + return type.typeOf(state.selectedDatas) === 'object' + ? state.selectedDatas[props.textField] + : state.selectedDatas🧰 Tools
🪛 eslint
[error] 101-101: Replace
·?·state.selectedDatas[props.textField]with⏎··········?·state.selectedDatas[props.textField]⏎·········(prettier/prettier)
packages/renderless/src/fluent-editor/index.ts (1)
2-2: Consider standardizing the import alias.The import uses
_typealias while other files usetypedirectly. Consider standardizing the alias usage across the codebase.-import { type as _type } from '@opentiny/utils' +import { type } from '@opentiny/utils'packages/renderless/src/picker/index.ts (1)
238-238: Fix the formatting of the ternary operator chain.The line needs to be formatted according to the project's style guide.
- let clientTimezoneOffset = type.isNumber(timezoneOffset) ? timezoneOffset : type.isNumber(TimezoneOffset) ? TimezoneOffset : 0 + let clientTimezoneOffset = type.isNumber(timezoneOffset) + ? timezoneOffset + : type.isNumber(TimezoneOffset) + ? TimezoneOffset + : 0🧰 Tools
🪛 eslint
[error] 238-238: Replace
·?·timezoneOffset·:·type.isNumber(TimezoneOffset)·?·TimezoneOffsetwith⏎······?·timezoneOffset⏎······:·type.isNumber(TimezoneOffset)⏎········?·TimezoneOffset⏎·······(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
.husky/pre-commit(1 hunks)examples/sites/demos/pc/webdoc/faq-en.md(1 hunks)examples/sites/demos/pc/webdoc/faq.md(1 hunks)packages/renderless/src/amount/index.ts(1 hunks)packages/renderless/src/base-select/index.ts(4 hunks)packages/renderless/src/base-select/vue.ts(2 hunks)packages/renderless/src/card/index.ts(2 hunks)packages/renderless/src/cascader-panel/index.ts(2 hunks)packages/renderless/src/cascader-panel/node.ts(2 hunks)packages/renderless/src/cascader/index.ts(4 hunks)packages/renderless/src/chart-bar/index.ts(5 hunks)packages/renderless/src/chart-candle/index.ts(4 hunks)packages/renderless/src/chart-core/deps/utils.ts(3 hunks)packages/renderless/src/chart-funnel/index.ts(2 hunks)packages/renderless/src/chart-gauge/index.ts(2 hunks)packages/renderless/src/chart-line/index.ts(3 hunks)packages/renderless/src/chart-map/index.ts(3 hunks)packages/renderless/src/chart-pie/index.ts(2 hunks)packages/renderless/src/chart-radar/index.ts(3 hunks)packages/renderless/src/chart-sankey/index.ts(2 hunks)packages/renderless/src/chart-scatter/index.ts(1 hunks)packages/renderless/src/chart-waterfall/index.ts(2 hunks)packages/renderless/src/checkbox/index.ts(4 hunks)packages/renderless/src/column-list-item/index.ts(2 hunks)packages/renderless/src/common/array.ts(3 hunks)packages/renderless/src/common/dataset/index.ts(3 hunks)packages/renderless/src/common/date.ts(7 hunks)packages/renderless/src/common/deps/date-util.ts(3 hunks)packages/renderless/src/common/deps/date.ts(4 hunks)packages/renderless/src/common/deps/dom.ts(5 hunks)packages/renderless/src/common/deps/popper.ts(3 hunks)packages/renderless/src/common/deps/tree-model/node.ts(3 hunks)packages/renderless/src/common/deps/tree-model/tree-store.ts(4 hunks)packages/renderless/src/common/deps/upload-ajax.ts(2 hunks)packages/renderless/src/common/function.ts(1 hunks)packages/renderless/src/common/object.ts(14 hunks)packages/renderless/src/common/runtime.ts(0 hunks)packages/renderless/src/common/string.ts(15 hunks)packages/renderless/src/common/validate/rules/range.ts(2 hunks)packages/renderless/src/common/validate/rules/required.ts(1 hunks)packages/renderless/src/common/validate/rules/type.ts(2 hunks)packages/renderless/src/common/validate/schema.ts(6 hunks)packages/renderless/src/common/validate/util.ts(3 hunks)packages/renderless/src/common/validate/validations/array.ts(1 hunks)packages/renderless/src/common/validate/validations/date.ts(1 hunks)packages/renderless/src/common/validate/validations/enum.ts(1 hunks)packages/renderless/src/common/validate/validations/float.ts(1 hunks)packages/renderless/src/common/validate/validations/integer.ts(1 hunks)packages/renderless/src/common/validate/validations/method.ts(1 hunks)packages/renderless/src/common/validate/validations/number.ts(1 hunks)packages/renderless/src/common/validate/validations/pattern.ts(1 hunks)packages/renderless/src/common/validate/validations/string.ts(1 hunks)packages/renderless/src/common/validate/validations/type.ts(1 hunks)packages/renderless/src/container/index.ts(2 hunks)packages/renderless/src/file-upload/index.ts(6 hunks)packages/renderless/src/file-upload/vue.ts(3 hunks)packages/renderless/src/fluent-editor/index.ts(2 hunks)packages/renderless/src/fluent-editor/options.ts(3 hunks)packages/renderless/src/form-item/index.ts(2 hunks)packages/renderless/src/grid/static/base/helperEqualCompare.ts(2 hunks)packages/renderless/src/grid/static/base/isPlainObject.ts(1 hunks)packages/renderless/src/grid/utils/common.ts(2 hunks)packages/renderless/src/image/index.ts(2 hunks)packages/renderless/src/modal/index.ts(2 hunks)packages/renderless/src/nav-menu/index.ts(3 hunks)packages/renderless/src/numeric/index.ts(5 hunks)packages/renderless/src/picker/index.ts(6 hunks)packages/renderless/src/popeditor/index.ts(4 hunks)packages/renderless/src/popup/index.ts(1 hunks)packages/renderless/src/recycle-scroller/index.ts(3 hunks)packages/renderless/src/rich-text/table-module.ts(8 hunks)packages/renderless/src/search/index.ts(3 hunks)packages/renderless/src/select/index.ts(4 hunks)packages/renderless/src/select/vue.ts(2 hunks)packages/renderless/src/skeleton/index.ts(1 hunks)packages/renderless/src/statistic/index.ts(2 hunks)packages/renderless/src/tabbar-item/index.ts(1 hunks)packages/renderless/src/toggle-menu/index.ts(2 hunks)packages/renderless/src/wizard/index.ts(2 hunks)packages/utils/src/type/index.ts(2 hunks)packages/vue-locale/package.json(1 hunks)packages/vue-locale/src/glob.ts(4 hunks)
⛔ Files not processed due to max files limit (11)
- packages/vue/src/grid-toolbar/package.json
- packages/vue/src/grid-toolbar/src/index.ts
- packages/vue/src/grid/src/body/src/body.tsx
- packages/vue/src/grid/src/footer/src/footer.ts
- packages/vue/src/grid/src/header/src/header.ts
- packages/vue/src/grid/src/table/src/methods.ts
- packages/vue/src/grid/src/table/src/table.ts
- packages/vue/src/grid/src/tools/formatter.ts
- packages/vue/src/image-viewer/src/mobileTouch.ts
- packages/vue/src/recycle-scroller/package.json
- packages/vue/src/recycle-scroller/src/idState.ts
💤 Files with no reviewable changes (1)
- packages/renderless/src/common/runtime.ts
✅ Files skipped from review due to trivial changes (1)
- .husky/pre-commit
👮 Files not reviewed due to content moderation or server errors (4)
- packages/renderless/src/common/deps/popper.ts
- packages/renderless/src/base-select/index.ts
- packages/renderless/src/select/index.ts
- packages/renderless/src/file-upload/index.ts
🧰 Additional context used
🪛 eslint
packages/renderless/src/picker/index.ts
[error] 238-238: Replace ·?·timezoneOffset·:·type.isNumber(TimezoneOffset)·?·TimezoneOffset with ⏎······?·timezoneOffset⏎······:·type.isNumber(TimezoneOffset)⏎········?·TimezoneOffset⏎·······
(prettier/prettier)
packages/renderless/src/popeditor/index.ts
[error] 101-101: Replace ·?·state.selectedDatas[props.textField] with ⏎··········?·state.selectedDatas[props.textField]⏎·········
(prettier/prettier)
packages/renderless/src/common/object.ts
[error] 232-232: Unexpected assignment within an 'if' statement.
(no-cond-assign)
packages/renderless/src/fluent-editor/options.ts
[error] 135-135: A constructor name should not start with a lowercase letter.
(new-cap)
🪛 Biome (1.9.4)
packages/renderless/src/common/validate/schema.ts
[error] 349-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/common/deps/date.ts
[error] 233-233: 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/chart-bar/index.ts
[error] 137-137: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 178-180: 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/chart-candle/index.ts
[error] 30-30: 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/cascader/index.ts
[error] 132-132: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (83)
packages/renderless/src/common/validate/validations/type.ts (2)
15-15: LGTM! Good practice to use centralized utilities.The change to import from
@opentiny/utilsaligns with the PR's objective of centralizing type utilities and improves maintainability.
15-20: Verify consistent usage of @opentiny/utils across validation files.Let's ensure all validation files have been updated consistently.
✅ Verification successful
Type utility migration successfully verified
All validation files have been consistently updated to use
@opentiny/utilsand no references to the old type module remain.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining direct imports from the old type module # and verify consistent usage of @opentiny/utils type utilities in validation files echo "Checking for remaining direct imports from type module..." rg -l "from '../../type'" "packages/renderless/src/common/validate/validations/" echo "Verifying consistent usage of @opentiny/utils..." rg -l "from '@opentiny/utils'" "packages/renderless/src/common/validate/validations/"Length of output: 1024
packages/renderless/src/skeleton/index.ts (1)
1-1: LGTM! Clean refactoring of type utilities.The changes correctly migrate from local type utilities to @opentiny/utils while maintaining the same functionality.
Also applies to: 4-4, 8-8
packages/renderless/src/statistic/index.ts (1)
1-1: LGTM! Import and usage updated correctly.The change correctly replaces the local type utility with the centralized
@opentiny/utilspackage, maintaining the same functionality while improving code organization.Also applies to: 12-12
packages/renderless/src/common/validate/validations/pattern.ts (1)
15-15: LGTM! Consistent with validation module changes.The update to use
type.hasOwnfrom@opentiny/utilsmaintains the same validation behavior while aligning with the centralized type utilities approach.Let's verify the consistency of this change across other validation files:
Also applies to: 19-19
✅ Verification successful
Verified: All validation files consistently use centralized type utilities
The change is consistent across all validation files:
- All individual validation files import
typefrom '@opentiny/utils'- Uniform usage of
type.hasOwn.callpattern- Only index.ts maintains local import as expected for module composition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining local type utility imports in validation files rg -l "from '../../type'" "packages/renderless/src/common/validate/validations/"Length of output: 86
Script:
#!/bin/bash # List all validation files echo "=== Validation Files ===" fd . "packages/renderless/src/common/validate/validations/" -e ts # Check all import patterns echo -e "\n=== Import Patterns ===" rg "import.*type" "packages/renderless/src/common/validate/validations/" # Check type utility usage echo -e "\n=== Type Utility Usage ===" rg "type\." "packages/renderless/src/common/validate/validations/"Length of output: 3770
packages/renderless/src/common/validate/validations/array.ts (1)
15-15: LGTM! The changes align with the PR objective.The replacement of the local type utility with
@opentiny/utilsmaintains the same validation behavior while centralizing type-checking utilities.Also applies to: 19-19
packages/renderless/src/common/validate/validations/number.ts (1)
15-15: LGTM! Changes are consistent with array.ts.The modifications follow the same pattern of centralizing type utilities while preserving the validation logic.
Also applies to: 19-19
packages/renderless/src/common/validate/validations/string.ts (1)
15-15: LGTM! Changes maintain consistency across validation files.The modifications align with the pattern seen in other validation files, ensuring consistent type utility usage while preserving the complex string validation logic.
Also applies to: 19-19
packages/renderless/src/tabbar-item/index.ts (1)
13-13: LGTM! Consistent usage of centralized type utilities.The changes appropriately replace local type utilities with the centralized ones from
@opentiny/utils. The functionality remains unchanged while improving code organization.Let's verify that we're using the new type utilities consistently:
Also applies to: 19-19, 21-21
✅ Verification successful
Huicharts module intentionally maintains its own type utilities
The remaining imports from
common/typein the huicharts module are correct as they use a specialized implementation with chart-specific type checking logic, including DOM-aware checks and detailed handling of edge cases. This is not an inconsistency with the migration to@opentiny/utils.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining direct imports from common/type rg -g '*.{ts,js}' "from.*common/type"Length of output: 241
Script:
#!/bin/bash # Check the implementation of type utilities in huicharts cat packages/vue/src/huicharts/huicharts-core/common/type.ts # Look for any README or documentation in huicharts fd README.md packages/vue/src/huicharts -x cat {}Length of output: 3461
packages/renderless/src/card/index.ts (1)
1-1: LGTM! Migration to centralized type utilities.The change correctly moves from local type utilities to
@opentiny/utils, maintaining the same functionality.Also applies to: 76-76
packages/renderless/src/grid/static/base/helperEqualCompare.ts (1)
26-26: LGTM! Consistent migration of type utilities.The changes correctly migrate type checks to the centralized utilities while preserving the original logic.
Since this is a utility function, let's verify its usage across the codebase:
Also applies to: 42-42, 46-46
✅ Verification successful
Migration of type utilities is safe and contained
The changes to
helperEqualCompare.tsare internal implementation details that don't affect its interface. The only consumerisEqual.tsuses it through a stable API that remains unchanged.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of helperEqualCompare to ensure the migration doesn't affect consumers rg -l "helperEqualCompare"Length of output: 141
Script:
#!/bin/bash # Check the implementation and usage in isEqual.ts cat packages/renderless/src/grid/static/base/isEqual.tsLength of output: 1399
packages/renderless/src/toggle-menu/index.ts (1)
13-13: LGTM! Proper migration of type utilities.The changes correctly migrate the object type check to the centralized utilities while maintaining the same functionality.
Also applies to: 51-51
packages/renderless/src/popup/index.ts (1)
16-16: LGTM! Consistent migration of type utilities.The changes correctly migrate the null check to the centralized utilities while preserving the original logic.
Also applies to: 21-21
packages/renderless/src/chart-sankey/index.ts (1)
15-15: LGTM! Type utility migration looks good.The migration from local type module to
@opentiny/utilsis done correctly. The null check functionality is preserved while maintaining the same behavior in the sankey chart's link value handling.Also applies to: 51-51
packages/renderless/src/chart-gauge/index.ts (1)
15-15: LGTM! Type utility migration looks good.The migration to
@opentiny/utilsis implemented correctly. The null check functionality for label mapping in the gauge chart is preserved while maintaining the same behavior.Also applies to: 61-61
packages/renderless/src/chart-funnel/index.ts (1)
15-15: LGTM! Type utility migration looks good.The migration to
@opentiny/utilsis implemented correctly. The null check functionality for legend name formatting in the funnel chart is preserved while maintaining the same behavior.Also applies to: 34-34
packages/renderless/src/common/dataset/index.ts (2)
14-14: LGTM! Import statement updated to use centralized type utilities.The change aligns with the PR objective of centralizing type-checking utilities.
59-59: Verify type-checking behavior remains consistent.The migration from local
isObjecttotype.isObjectaffects object type checking in two critical paths:
- Recursive property access in
getNsObj- Source configuration validation in
getDatasetLet's verify the usage patterns:
Also applies to: 112-112
✅ Verification successful
Type checking behavior remains consistent after migration
The migration from local
isObjecttotype.isObjectmaintains consistent behavior as it uses the standardized type utility implementation that's uniform across the codebase. The implementation(obj: any) => typeOf(obj) === 'object'is consistently used in both critical paths: recursive property access and source configuration validation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for existing isObject usage patterns to ensure consistent behavior rg "isObject\(" --type tsLength of output: 6752
Script:
#!/bin/bash # Check the implementation of isObject in type utility ast-grep --pattern 'export const isObject = $_' # Also check for any local implementation ast-grep --pattern 'const isObject = $_'Length of output: 1406
packages/vue-locale/src/glob.ts (1)
3-3: LGTM! Import statement updated to use centralized type utilities.The change aligns with the PR objective of centralizing type-checking utilities.
packages/renderless/src/common/validate/rules/type.ts (2)
17-17: LGTM! Import statement updated to use centralized type utilities.The change aligns with the PR objective of centralizing type-checking utilities.
51-54: Consider adding unit tests for type validation edge cases.The changes affect core type validation rules. Ensure edge cases are covered:
- Date objects with invalid dates
- Numbers at type boundaries
- Objects with null prototypes
- Methods with bound contexts
Let's check existing test coverage:
packages/renderless/src/grid/utils/common.ts (1)
26-26: LGTM! Clean migration to centralized type utilities.The change correctly moves the type check to the shared utility package while maintaining the same functionality.
Also applies to: 87-87
packages/renderless/src/cascader-panel/node.ts (1)
21-21: LGTM! Well-maintained TypeScript integration.The migration to centralized type utilities is clean and maintains proper TypeScript type definitions.
Also applies to: 83-83
packages/renderless/src/common/array.ts (1)
15-15: LGTM! Improved type safety in array operations.The migration to centralized type utilities and use of strict equality improves code reliability.
Also applies to: 126-126
packages/renderless/src/chart-core/deps/utils.ts (2)
14-14: LGTM! Consistent import of centralized utilities.The change aligns with the PR objective of centralizing type utilities.
27-27: LGTM! Consistent usage of centralized type utilities.The changes consistently replace local type utilities with their centralized counterparts from @opentiny/utils, improving maintainability.
Also applies to: 45-45, 205-205, 211-211, 276-276
packages/renderless/src/image/index.ts (1)
15-15: LGTM! Consistent type checking implementation.The changes appropriately replace the local typeOf utility with the centralized type.typeOf from @opentiny/utils.
Also applies to: 109-109
packages/renderless/src/common/validate/util.ts (1)
13-13: LGTM! Consistent utility function migration.The changes successfully migrate from local type utilities to the centralized @opentiny/utils package.
Also applies to: 112-112, 276-276
packages/renderless/src/checkbox/index.ts (1)
14-14: LGTM! Consistent null check implementation.The changes successfully migrate all null checks to use the centralized type.isNull utility from @opentiny/utils.
Also applies to: 75-75, 107-107, 212-212, 217-217
packages/renderless/src/chart-scatter/index.ts (1)
16-16: LGTM! Type utility migration looks good.The changes correctly migrate the type checking to use the centralized
@opentiny/utilspackage.Also applies to: 20-20
packages/renderless/src/common/deps/dom.ts (3)
13-14: LGTM! Property check migration looks good.The migration from
hasOwntotype.hasOwnis correctly implemented.Also applies to: 129-129
152-152: LGTM! Null check migration looks good.The migration of null checks to use
type.isNullis consistent and correct.Also applies to: 198-198
224-224: Verify global config compatibility.The change from
globalConfigtoglobal.globalConfigmight affect existing code that relies on the global configuration.packages/renderless/src/common/deps/date.ts (2)
14-14: LGTM!The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
276-276: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities approach.packages/renderless/src/common/deps/date-util.ts (3)
16-16: LGTM!The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
42-42: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities approach.
213-213: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities approach.packages/renderless/src/common/validate/schema.ts (4)
14-14: LGTM!The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
Line range hint
23-23: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities approach.
Line range hint
48-48: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities approach.
371-371: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities approach.packages/renderless/src/common/deps/tree-model/tree-store.ts (4)
15-15: LGTM!The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
23-23: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities approach.
251-251: LGTM!The change to use
type.hasOwn.callmaintains the same functionality while aligning with the centralized type utilities approach.
381-381: LGTM!The change to use
type.isNullmaintains the same functionality while aligning with the centralized type utilities approach.packages/renderless/src/amount/index.ts (1)
327-331: LGTM! Consistent type checking implementation.The changes standardize type checking by using the centralized
type.isNumberutility from@opentiny/utils. This enhances code consistency and maintainability.Also applies to: 336-336
packages/renderless/src/chart-bar/index.ts (1)
158-161: LGTM! Consistent type checking implementation.The changes standardize type checking by using the centralized
type.isNullutility from@opentiny/utils.packages/renderless/src/common/object.ts (1)
255-255: LGTM! Consistent type checking implementation.The changes standardize type checking by using the centralized
typeutility from@opentiny/utils. This enhances code consistency and maintainability across the codebase.Also applies to: 261-261, 268-268, 424-424
packages/renderless/src/cascader-panel/index.ts (1)
93-93: LGTM! Consistent type checking implementation.The change standardizes null checking by using the centralized
type.isNullutility from@opentiny/utils.packages/renderless/src/file-upload/vue.ts (1)
101-101: LGTM! Standardizing utility functions.The changes correctly migrate to using the centralized
typeutility from@opentiny/utils, improving consistency across the codebase.Also applies to: 180-180, 197-197
packages/renderless/src/rich-text/table-module.ts (1)
2-2: LGTM! Consistent migration to centralized utilities.The changes successfully migrate all
isNullchecks to use the standardizedtype.isNullutility from@opentiny/utils, maintaining the original logic while improving code consistency.Also applies to: 136-136, 223-223, 261-261, 341-341, 373-373, 430-430, 445-445
packages/renderless/src/common/deps/tree-model/node.ts (1)
16-16: LGTM! Proper migration to standardized type utilities.The changes successfully migrate both
typeOfandhasOwnchecks to use the standardized utilities from@opentiny/utils, maintaining the original functionality while improving code consistency.Also applies to: 26-26, 28-28, 164-164
packages/renderless/src/common/date.ts (1)
13-13: LGTM! Comprehensive migration to standardized type utilities.The changes successfully migrate all type-checking utilities (
isNumber,isDate,isNumeric) to use their standardized counterparts from@opentiny/utils, maintaining the original functionality while improving code consistency.Also applies to: 363-363, 407-407, 458-458, 482-482, 485-485, 505-505, 532-532
packages/renderless/src/form-item/index.ts (1)
17-17: LGTM! Consistent type checking implementation.The change to use
type.isNullfrom@opentiny/utilsaligns with the PR objective of centralizing type-checking utilities.Also applies to: 33-33
packages/renderless/src/numeric/index.ts (1)
24-25: LGTM! Consistent type checking implementation.The changes effectively replace local type checking with centralized utilities from
@opentiny/utils, maintaining functionality while improving code organization.Also applies to: 204-204, 391-399
packages/renderless/src/recycle-scroller/index.ts (1)
2-2: LGTM! Consistent type checking implementation.The changes effectively replace local type checking with centralized utilities from
@opentiny/utils, maintaining the same error checking functionality.Also applies to: 491-491, 543-543
packages/renderless/src/base-select/vue.ts (1)
103-103: LGTM! Consistent type checking implementation.The changes effectively replace local type checking with centralized utilities from
@opentiny/utils, maintaining the same functionality in the debounce computation.Also applies to: 268-268
packages/renderless/src/nav-menu/index.ts (3)
21-21: LGTM! Import statement aligns with centralization goal.The addition of the
typeutility import from '@opentiny/utils' is consistent with the PR's objective of centralizing type-checking utilities.
44-47: LGTM! Type checking updated correctly.The transition to
type.isEmptyObjectfrom the centralized utility package maintains the same logic while improving consistency across the codebase.
185-185: LGTM! Type checking updated correctly.The transition to
type.isObjectfrom the centralized utility package maintains the same logic while improving consistency across the codebase.packages/renderless/src/cascader/index.ts (1)
14-14: LGTM! Import statement aligns with centralization goal.The addition of the
typeutility import from '@opentiny/utils' is consistent with the PR's objective.packages/renderless/src/select/vue.ts (2)
116-116: LGTM! Import statement aligns with centralization goal.The addition of the
typeutility import from '@opentiny/utils' is consistent with the PR's objective.
326-326: LGTM! Type checking updated correctly.The transition to
type.isNumberfrom the centralized utility package maintains the same logic while improving consistency across the codebase.packages/renderless/src/common/string.ts (1)
224-224: LGTM! Comprehensive update of type checking utilities.All type-checking operations have been successfully migrated to use the centralized
typeutility from '@opentiny/utils'. This change:
- Maintains the same logic while improving code consistency
- Reduces code duplication
- Makes the codebase more maintainable
Also applies to: 369-369, 395-395, 466-466, 485-485, 534-534, 558-558, 589-600, 652-652, 692-694, 717-719, 735-735, 781-781
packages/renderless/src/modal/index.ts (2)
17-17: LGTM! Import of global utility.The import of
globalfrom@opentiny/utilsaligns with the codebase's utility standardization effort.
282-282: Verify viewport window access across different environments.The
getViewportWindowfunction is used to access viewport dimensions. Ensure it works correctly in different environments (iframe, shadow DOM, etc.).Run this script to check for potential viewport access issues:
✅ Verification successful
Viewport window access is properly handled across environments
The
getViewportWindow()implementation is specifically designed for cross-environment compatibility:
- Configurable viewport window with proper fallbacks
- Explicit support for micro-frontends and complex DOM contexts
- Consistent access patterns across components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for all usages of getViewportWindow to verify consistent access patterns # and potential environment-specific handling # Check for any special viewport handling in different environments rg -A 5 "getViewportWindow|document\.documentElement|document\.body"Length of output: 70177
Script:
#!/bin/bash # Search for tests and configurations related to getViewportWindow rg -A 5 "getViewportWindow|viewportWindow" "**/*.test.{ts,js,tsx,jsx}"Length of output: 138
Script:
#!/bin/bash # Search for modal tests and viewport window related code rg -A 5 "getViewportWindow|viewportWindow|globalConfig" "packages/**/*modal*/**/*.{ts,js,tsx,jsx}"Length of output: 181
Script:
#!/bin/bash # Search for viewport window related code and tests with simpler paths rg -A 5 "getViewportWindow|viewportWindow|globalConfig.viewportWindow" packages/Length of output: 8919
packages/renderless/src/fluent-editor/options.ts (2)
1-1: LGTM! Import of type utility.The import of
typefrom@opentiny/utilsaligns with the codebase's utility standardization effort.
105-144: LGTM! List autofill handler implementation.The function correctly handles various list formats and includes proper null checks using
type.isNull. The implementation properly handles:
- Table cell line formats
- List query checks
- Different list prefixes ([], [ ], [x], -, *)
🧰 Tools
🪛 eslint
[error] 135-135: A constructor name should not start with a lowercase letter.
(new-cap)
packages/renderless/src/popeditor/index.ts (1)
14-14: LGTM! Import of type utility.The import of
typefrom@opentiny/utilsaligns with the codebase's utility standardization effort.packages/renderless/src/fluent-editor/index.ts (1)
296-296: LGTM! Null check in file input handler.The null check using
_type.isNullis correctly implemented before creating and configuring the file input element.packages/vue-locale/package.json (1)
11-11: LGTM! Dependency addition aligns with PR objectives.The addition of
@opentiny/utilsas a workspace dependency is consistent with the goal of centralizing utility functions.packages/renderless/src/common/validate/validations/method.ts (1)
15-15: LGTM! Verify other usages of hasOwn in validation files.The migration from local type utility to
@opentiny/utilsis consistent. Let's verify other validation files for similar patterns.Also applies to: 18-18
✅ Verification successful
✅ All hasOwn usages in validation files are consistent
All validation files are already using the
type.hasOwn.callpattern from@opentiny/utils. No remaining direct imports or inconsistent usages found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining direct hasOwn imports in validation files rg -l "import.*hasOwn.*from.*type" "packages/renderless/src/common/validate" # Search for any remaining direct hasOwn calls rg "hasOwn\.call" "packages/renderless/src/common/validate" --no-filenameLength of output: 1482
packages/renderless/src/common/validate/validations/enum.ts (1)
15-15: LGTM! Consistent with method.ts changes.The migration pattern matches the changes in method.ts, maintaining consistency across validation files.
Also applies to: 21-21
examples/sites/demos/pc/webdoc/faq.md (1)
11-11: Verify backward compatibility of global configuration changes.While the migration to
@opentiny/utilsis aligned with PR objectives, let's ensure this change doesn't break existing micro-frontend implementations.Also applies to: 16-16
✅ Verification successful
Import change maintains backward compatibility ✅
The migration to
@opentiny/utilspreserves theglobal.globalConfigstructure and all existing usages are being updated consistently.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other globalConfig usages that might need similar updates rg -l "import.*globalConfig.*from.*@opentiny/vue-renderless/common/global" . # Search for direct globalConfig assignments rg "globalConfig\.viewportWindow\s*=" .Length of output: 309
examples/sites/demos/pc/webdoc/faq-en.md (1)
11-11: LGTM! Changes are synchronized with Chinese version.The updates maintain consistency between language versions of the documentation.
Also applies to: 16-16
packages/renderless/src/chart-pie/index.ts (2)
15-15: LGTM! Import statement updated correctly.The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
126-126: LGTM! Type check implementation is correct.The
type.isNullusage maintains the same behavior while leveraging the centralized utility.packages/renderless/src/picker/index.ts (5)
14-14: LGTM! Import statement updated correctly.The change aligns with the PR objective of centralizing type utilities by importing from
@opentiny/utils.
200-202: LGTM! Type check implementation is correct.The
type.isDateusage in the array mapping maintains the same behavior while leveraging the centralized utility.
216-219: LGTM! Type checks implemented correctly.The
type.isNumberusage in value transformation maintains the same behavior while leveraging the centralized utility.
257-257: LGTM! Type check implementation is correct.The
type.isNumberusage in the array validation maintains the same behavior while leveraging the centralized utility.
1370-1370: LGTM! Type check implementation is correct.The
type.isDateusage in the date transformation maintains the same behavior while leveraging the centralized utility.
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
packages/renderless/src/scrollbar/vue-bar.ts (1)
Line range hint
47-53: Fix inconsistent usage of DOM utilities.The code inconsistently uses DOM utilities:
- Line 47 uses
dom.off- Line 51 and 53 still use the direct
onandoffreferences which are no longer importedApply this diff to fix the inconsistency:
onUnmounted(() => dom.off(document, 'mouseup', api.mouseUpDocumentHandler)) return Object.assign(api, { - startDrag: startDrag({ api, on, state }), + startDrag: startDrag({ api, on: dom.on, state }), clickThumbHandler: clickThumbHandler({ api, state }), - mouseUpDocumentHandler: mouseUpDocumentHandler({ api, off, state }) + mouseUpDocumentHandler: mouseUpDocumentHandler({ api, off: dom.off, state }) })packages/renderless/src/steps/slide-bar.ts (1)
Documentation updates needed for DOM utilities migration
The migration to
@opentiny/utilsrequires documentation updates in the following locations:
packages/utils/README.md: Add documentation for DOM utilities and their usagepackages/renderless/README.md: Update with examples showing the preferred way to handle DOM operations using@opentiny/utilsThis documentation is important as
@opentiny/utilsis a relatively new package (as seen in the changelog) and developers need clear guidance on using its DOM utilities.🔗 Analysis chain
Line range hint
2-158: Documentation updates needed.While the code changes look good, please update the documentation to reflect the migration to
@opentiny/utils. This will help maintain consistency in the documentation and make it easier for other developers to understand the preferred way of using DOM utilities.Let's check for any existing documentation that needs updating:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for documentation files that might need updates fd -e md -e mdx . | xargs rg -l "on\s*\(\s*window|off\s*\(\s*window"Length of output: 70
Script:
#!/bin/bash # Search for any documentation mentioning @opentiny/utils rg -l "@opentiny/utils" --type md --type mdx # Search for DOM-related documentation rg -l "DOM|dom util" --type md --type mdx # Check for README files in the packages directory fd "README.md" packages/Length of output: 560
Script:
#!/bin/bash # Check content of utils and renderless README files cat packages/utils/README.md cat packages/renderless/README.md # Retry searching for @opentiny/utils mentions with corrected flags rg -l "@opentiny/utils" -g "*.md" # Search for DOM-related content with corrected flags rg -l "DOM|dom util" -g "*.md"Length of output: 896
Script:
#!/bin/bash # Check changelog and FAQ content for context about @opentiny/utils rg -A 5 "@opentiny/utils" examples/sites/demos/pc/webdoc/changelog.md examples/sites/demos/pc/webdoc/faq.md # Check DOM-related content in changelog rg -A 5 "DOM|dom util" examples/sites/demos/pc/webdoc/changelog-en.mdLength of output: 2372
🧹 Nitpick comments (45)
packages/renderless/src/dropdown-item/index.ts (1)
Line range hint
1-186: Consider adding JSDoc documentation for exported functions.While the code changes look good, consider adding JSDoc documentation for the exported functions to improve maintainability and developer experience. This would be particularly helpful for functions that handle DOM events and user interactions.
Example documentation format:
/** * Binds or unbinds scroll event listener to the parent's scroller * @param value - Boolean indicating whether to bind (true) or unbind (false) the scroll event */ export const bindScroll = ...packages/renderless/src/scrollbar/vue-bar.ts (1)
Line range hint
1-53: Consider adding JSDoc documentation.Based on the PR objectives which mention missing documentation updates, consider adding JSDoc comments for the exported functions and interfaces to improve maintainability.
Add documentation like this:
+/** + * Renderless API for the scrollbar's bar component + * @type {string[]} + */ export const api = [ 'state', 'startDrag', // ... ] +/** + * Renderless implementation of the scrollbar's bar component + * @param {Object} props - Component props + * @param {Object} context - Setup context containing computed, lifecycle hooks, etc. + * @param {Object} extras - Extra context containing vm and parent references + * @returns {Object} Component's renderless API + */ export const renderless = (props, { computed, onUnmounted, reactive }, { vm, parent }) => {packages/renderless/src/tall-storage/vue.ts (1)
Line range hint
1-44: Add JSDoc documentation for the exported API.While the code changes look good, the PR objectives mention that documentation updates are missing. Consider adding JSDoc documentation for:
- The exported
apiarray- The
renderlessfunction and its parameters- The state structure and event handlers
Example documentation:
/** * Exported API surface for the tall-storage renderless component */ export const api = ['state', 'mousedown', 'selectItem'] /** * Renderless component logic for tall-storage * @param props Component props * @param context Vue composition API context containing lifecycle hooks and reactive utilities * @param emit Event emitter function * @returns Component API containing state and event handlers */ export const renderless = (props, { onUnmounted, reactive, watch }, { emit }) => { // ... }packages/vue-common/src/breakpoint.ts (3)
Line range hint
16-23: Add type assertion for better type safety.While the server-side check is correct, adding a type assertion would improve type safety and maintainability.
Consider applying this change:
- return { + return { matches: false, media: mediaQueryString, addEventListener: () => {}, removeEventListener: () => {} - } + } as MediaQueryList } else {
Line range hint
43-43: Replace 'any' type with proper event listener type.The explicit 'any' type for
mediaQueryListenercan be replaced with a more specific type.Consider applying this change:
- const mediaQueryListener: any = debounce(0, () => setActiveBreakpoint()) + const mediaQueryListener: EventListener = debounce(0, () => setActiveBreakpoint())
Line range hint
6-11: Documentation needs updating to reflect utility package changes.The JSDoc comments are good but should be updated to mention the utility package dependency, especially since documentation updates were marked as incomplete in the PR checklist.
Consider adding a note about the required
@opentiny/utilspackage in the documentation.packages/renderless/src/grid/utils/dom.ts (2)
312-312: Add newline at end of file.ESLint requires files to end with a newline character.
}) +🧰 Tools
🪛 eslint
[error] 312-312: Insert
⏎(prettier/prettier)
Line range hint
1-312: Consider adding TypeScript type definitions.The file would benefit from TypeScript type definitions for:
- Function parameters and return types
- DOM element types
- Event types
This would improve maintainability and provide better IDE support.
packages/renderless/src/pull-refresh/index.ts (1)
Line range hint
13-157: Consider these improvements for better maintainability.While the changes look good, here are some suggestions for future improvements:
- Consider making
PULL_UP_TIME_OUTconfigurable via props for better flexibility.- The scroll position calculations in
pullUpTouchEndcould be abstracted into a utility function for better reusability.- Consider consolidating the success/error text handling into a single function to reduce code duplication.
packages/renderless/src/slider/index.ts (1)
122-125: Consider adding touch event error handling.While the event binding changes look good, consider adding error handling for touch events to gracefully handle cases where
touches[0]might be undefined (e.g., when all touches are removed).dom.on(window, 'touchend', api.bindMouseUp) -dom.on(window, 'touchmove', api.bindMouseMove) +dom.on(window, 'touchmove', (event) => { + try { + api.bindMouseMove(event) + } catch (error) { + // Handle the case where touches[0] is undefined + api.bindMouseUp() + } +})packages/renderless/src/pull-refresh-mobile-first/vue.ts (1)
Line range hint
38-38: Consider moving the emulate() call inside a function.The emulate() call at the module level could have side effects during module initialization. Consider moving it inside the renderless function or a setup function for better control over when it's executed.
-emulate() + export const api = ['state', 'constants', 'onTouchStart', 'onTouchEnd', 'getStatusText'] export const renderless = ( props, { ref, unref, toRef, isRef, reactive, computed, watch, onMounted, onUnmounted, onActivated, onDeactivated }, { vm, emit, constants, t, nextTick, slots }, { Loading } ) => { + emulate() const api = {}packages/renderless/src/common/deps/repeat-click.ts (2)
35-43: Consider adding cleanup, touch support, and accessibility improvements.While the implementation works for mouse events, there are several areas for improvement:
- Add cleanup logic to prevent memory leaks when the component is destroyed
- Add touch event support for mobile devices
- Add keyboard event support for accessibility
- Handle edge cases when mouseup occurs outside the document
Here's a suggested implementation:
export default (el, binding) => { const LONG_PRESS_INTERVAL = 200 let interval = null let startTime + let cleanup = null const handler = () => { typeof binding.value === 'function' && binding.value.apply() } const clear = () => { if (Date.now() - startTime < LONG_PRESS_INTERVAL) { handler() } clearInterval(interval) interval = null + cleanup = null } - dom.on(el, 'mousedown', (e) => { + const start = (e) => { if (e.button !== 0) { return } startTime = Date.now() - dom.once(document, 'mouseup', clear) + cleanup = () => { + dom.off(document, 'mouseup', clear) + dom.off(document, 'touchend', clear) + clear() + } + dom.on(document, 'mouseup', clear) + dom.on(document, 'touchend', clear) clearInterval(interval) interval = setInterval(handler, LONG_PRESS_INTERVAL) - }) + } + + // Mouse events + dom.on(el, 'mousedown', start) + + // Touch events + dom.on(el, 'touchstart', start) + + // Keyboard events + dom.on(el, 'keydown', (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + handler() + } + }) + + // Cleanup when directive is unmounted + return () => { + dom.off(el, 'mousedown', start) + dom.off(el, 'touchstart', start) + cleanup?.() + clearInterval(interval) + } }
16-16: Improve documentation for LONG_PRESS_INTERVAL constant.The comment "fix issue#919" doesn't provide enough context. Please add documentation explaining:
- Why 200ms was chosen as the interval
- What issue#919 was about and how this fixes it
- Any implications of modifying this value
packages/renderless/src/file-upload/index.ts (1)
Line range hint
1-2482: Consider breaking down this large module into smaller, focused modules.While the current changes are good, the file could benefit from some architectural improvements:
Split the functionality into separate modules:
- Upload handling
- Download handling
- Preview handling
- EDM integration
- Type checking and validation
Consider extracting common utilities:
- File chunking logic
- Progress tracking
- Error handling
- Token management
This would improve maintainability, testability, and make the code easier to understand.
packages/vue/src/grid/src/composable/useDrag/dnd.ts (2)
87-91: Consider adding error handling for event listener registration.While the refactoring to use
dom.onis good, consider adding error handling to gracefully handle cases where event registration fails.- dom.on(dndProxyElement, 'dragstart', onDragStart) - dom.on(dndProxyElement, 'dragover', onDragOver) - dom.on(dndProxyElement, 'dragenter', onDragEnter) - dom.on(dndProxyElement, 'dragend', onDragEnd) - dom.on(dndProxyElement, 'drop', onDrop) + try { + dom.on(dndProxyElement, 'dragstart', onDragStart) + dom.on(dndProxyElement, 'dragover', onDragOver) + dom.on(dndProxyElement, 'dragenter', onDragEnter) + dom.on(dndProxyElement, 'dragend', onDragEnd) + dom.on(dndProxyElement, 'drop', onDrop) + } catch (error) { + console.error('Failed to register drag and drop event listeners:', error) + // Consider cleanup of any successfully registered listeners + destroy() + }
251-255: Consider adding error handling for event listener cleanup.While the refactoring to use
dom.offis good, consider adding error handling to ensure all listeners are properly removed even if some cleanup operations fail.- dom.off(dndProxyElement, 'dragstart', onDragStart) - dom.off(dndProxyElement, 'dragover', onDragOver) - dom.off(dndProxyElement, 'dragenter', onDragEnter) - dom.off(dndProxyElement, 'dragend', onDragEnd) - dom.off(dndProxyElement, 'drop', onDrop) + try { + dom.off(dndProxyElement, 'dragstart', onDragStart) + dom.off(dndProxyElement, 'dragover', onDragOver) + dom.off(dndProxyElement, 'dragenter', onDragEnter) + dom.off(dndProxyElement, 'dragend', onDragEnd) + dom.off(dndProxyElement, 'drop', onDrop) + } catch (error) { + console.error('Failed to cleanup some drag and drop event listeners:', error) + // Continue with remaining cleanup regardless of listener removal failures + }packages/renderless/src/top-box/index.ts (1)
39-39: LGTM! Consider adding JSDoc for event handling.The change to
dom.off()is correct and consistent with the correspondingbindEventchange. Consider adding JSDoc comments to document the event handling lifecycle.Add documentation above the event handling functions:
+/** + * Binds keydown event handler to document + * @param {Object} api - Component API object + */ export const bindEvent = ({ api }) => () => dom.on(document, 'keydown', api.bindKeyDown) +/** + * Removes keydown event handler from document + * @param {Object} api - Component API object + */ export const unBindEvent = ({ api }) => () => dom.off(document, 'keydown', api.bindKeyDown)packages/renderless/src/popeditor/index.ts (3)
781-784: Consider using a cleanup function for event listeners.While the DOM event handling is correctly updated to use the utility functions, the event listeners should be properly cleaned up to prevent memory leaks.
Consider wrapping the event listeners in a cleanup function:
+const attachEventListeners = ({ api }) => { + dom.on(document, 'click', api.closeSuggestPanel) + dom.on(window, 'resize', api.updateSuggestWidth) +} + +const detachEventListeners = ({ api }) => { + dom.off(window, 'resize', api.updateSuggestWidth) + dom.off(document, 'click', api.closeSuggestPanel) +} + export const doSuggesst = ({ state, props, popper, api, nextTick }) => (event) => { if (!props.suggest) { return } let query = event - dom.off(window, 'resize', api.updateSuggestWidth) - dom.off(document, 'click', api.closeSuggestPanel) - dom.on(document, 'click', api.closeSuggestPanel) - dom.on(window, 'resize', api.updateSuggestWidth) + detachEventListeners({ api }) + attachEventListeners({ api })
842-843: Use the same cleanup function for consistency.For consistency with the earlier event handling, use the same cleanup function here.
export const closeSuggestPanel = ({ state, api, vm }) => (event = false) => { // ... if (!keep) { - dom.off(document, 'click', api.closeSuggestPanel) - dom.off(window, 'resize', api.updateSuggestWidth) + detachEventListeners({ api })
Line range hint
1-1000: Add return type annotations for better type safety.The file contains numerous exported functions without TypeScript return type annotations. Consider adding them to improve type safety and documentation.
Example for a few functions:
-export const computedGetTitle = ({ constants, props, t }) => +export const computedGetTitle = ({ constants, props, t }): string => -export const computedModalWidth = ({ constants, props }) => +export const computedModalWidth = ({ constants, props }): string => -export const createSearchForm = (props) => (isRest) => +export const createSearchForm = (props) => (isRest: boolean): Record<string, string> =>packages/renderless/src/common/deps/fullscreen/apis.ts (2)
139-140: Consider consolidating the keypressCallback setup.While the changes correctly use the new dom utility, consider moving the
keypressCallbacksetup before its first usage at line 131. This would improve code organization and prevent potential undefined behavior.if (this.opts.pageOnly) { + // Setup keypress callback for page-only fullscreen mode + const keypressCallback = (e) => { + if (e.key === 'Escape') { + dom.off(document, 'keyup', keypressCallback) + this.exit() + } + } + + dom.off(document, 'keyup', keypressCallback) + dom.on(document, 'keyup', keypressCallback) + this.isFullscreen = true this.targetElement = targetEle - // Network page fullscreen mode keypress callback - const keypressCallback = (e) => { - if (e.key === 'Escape') { - dom.off(document, 'keyup', keypressCallback) - this.exit() - } - } - - dom.off(document, 'keyup', keypressCallback) - dom.on(document, 'keyup', keypressCallback) if (this.opts.callback) {
Line range hint
1-193: Consider adding JSDoc documentation for the exported API.While the code changes look good, consider adding JSDoc documentation for the exported
apiobject and its methods. This would help users understand the available options, their effects, and usage patterns.packages/renderless/src/load-list/index.ts (1)
1-1: Add newline after import statement.The migration to
@opentiny/utilsaligns well with the PR's objective of centralizing utility functions. However, there's a minor style issue to fix.import { dom } from '@opentiny/utils' + export const clickList =🧰 Tools
🪛 eslint
[error] 1-1: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
packages/renderless/src/signature/index.ts (1)
Line range hint
94-124: Consider improving error handling and event cleanup.The touch event handling implementation could benefit from several improvements:
- Add proper error handling for canvas context acquisition
- Avoid relying on the global
eventobject- Consider cleaning up any remaining state in
touchEndHere's a suggested approach:
// Add a type-safe context check const getContext = (canvas: HTMLCanvasElement | null): CanvasRenderingContext2D => { if (!canvas) { throw new Error('Canvas element not found') } const context = canvas.getContext('2d') if (!context) { throw new Error('Failed to get canvas context') } return context } // Example usage in touchMove export const touchMove = ({ emit, state }) => (event: TouchEvent) => { try { if (!state.ctx) { throw new Error('Canvas context not initialized') } dom.preventDefault(event) // Use the event parameter instead of global event object const touch = event.touches[0] // ... rest of the implementation } catch (error) { emit('error', error) return false } } // Add cleanup in touchEnd export const touchEnd = ({ emit, state }) => (event: TouchEvent) => { state.isDrawing = false dom.preventDefault(event) // Clean up any temporary state state.ctx?.closePath() emit('end') }packages/renderless/src/sticky/vue.ts (1)
Line range hint
1-85: Consider adding documentation for the sticky positioning logic.Since the PR checklist indicates missing documentation updates, consider adding:
- JSDoc comments for the exported functions
- Inline comments explaining the sticky positioning calculations
- Usage examples in the package documentation
packages/renderless/src/notify/index.ts (1)
93-93: LGTM! Consider adding JSDoc comments.The change to use
dom.offmaintains proper cleanup and symmetry with the bind operation. Consider adding JSDoc comments to document the function's purpose and parameters.+/** + * Removes the keydown event listener from the document. + * @param {Object} api - The API object containing the bindKeyDown method. + * @returns {Function} Cleanup function that removes the event listener. + */ export const unBindEvent = (api) => () => dom.off(document, 'keydown', api.bindKeyDown)packages/renderless/src/floating-button/index.ts (2)
38-40: Consider consolidating scroll event handlers.While the event cleanup is correctly implemented, there are two separate scroll event handlers (
api.onScrollandapi.getScrollListener). Consider consolidating these to:
- Improve performance by reducing event listeners
- Simplify the cleanup process
- Prevent potential race conditions
export const clearTimer = ({ state, api }) => () => { clearTimeout(state.initTimer) dom.off(window, 'resize', api.getClientWidth) - dom.off(window, 'scroll', api.onScroll) - dom.off(window, 'scroll', api.getScrollListener) + dom.off(window, 'scroll', api.handleScroll) // Combined scroll handler }
150-150: Consider centralizing all event bindings in mounted.While the resize event binding is correct, consider centralizing all event bindings (including scroll) in the mounted function for better maintainability and clearer lifecycle management.
export const mounted = (api) => () => { api.getClientWidth() api.onScroll() api.getExpandList() dom.on(window, 'resize', api.getClientWidth) + dom.on(window, 'scroll', api.handleScroll) // Combined scroll handler }packages/renderless/src/loading/index.ts (1)
Line range hint
1-48: Add documentation for the utility migration.The PR checklist indicates that documentation updates are missing. Consider adding documentation to help other developers understand:
- The migration to centralized utilities
- Any breaking changes in the utility functions' behavior
- Migration guide for other teams
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/tree/index.ts (1)
Line range hint
1-824: Consider adding JSDoc comments for better documentation.The file contains complex tree manipulation logic but lacks comprehensive documentation. Consider adding JSDoc comments to describe the purpose and parameters of key functions.
Example for one of the functions:
+/** + * Handles the drag start event for tree nodes + * @param {DragEvent} event - The drag event + * @param {TreeNode} treeNode - The node being dragged + * @returns {boolean} False if drag is not allowed + */ export const dragStart = ({ props, state, emit }) => (event, treeNode) => {packages/renderless/src/common/deps/popup-manager.ts (2)
107-107: Consider batching class operations for better performance.While the changes correctly use the centralized DOM utilities, consider batching multiple class operations together for better performance, especially in the loop at line 115.
- classArr.forEach((cls) => doms.addClass(modalDom, cls)) + doms.addClass(modalDom, classArr.join(' '))Also applies to: 110-110, 115-115, 119-119
166-166: Apply consistent class operation batching here as well.For consistency with the previous suggestion and better performance, consider batching the class operations here too.
- classArr.forEach((cls) => doms.removeClass(modalDom, cls)) + doms.removeClass(modalDom, classArr.join(' '))Also applies to: 182-182, 183-183, 196-196
packages/utils/src/index.ts (2)
8-8: Consider maintaining alphabetical order in exports.For better readability and maintainability, consider ordering the exports alphabetically:
-export { xss, logger, crypt, dom, global, type } +export { crypt, dom, global, logger, type, xss }
13-16: Maintain consistent ordering between named and default exports.The ordering in the default export object differs from the named exports above. Consider using the same ordering in both places for consistency and easier maintenance.
export default { xss, logger, - dom, crypt, + dom, global, type }packages/renderless/src/common/deps/useEventListener.ts (1)
1-1: Add a newline after the import statement.To maintain consistent code style and comply with the project's ESLint rules, add an empty line after the import statement.
import { dom } from '@opentiny/utils' + export const onMountedOrActivated =🧰 Tools
🪛 eslint
[error] 1-1: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
packages/renderless/src/nav-menu/index.ts (2)
Line range hint
4-4: Add test coverage for the nav-menu component.The TODO comment indicates missing tests. Consider adding comprehensive test coverage for:
- Menu item rendering
- Event handling
- URL handling and security
- Responsive behavior
Would you like me to help create a test suite for this component?
Line range hint
472-486: Consider enhancing error handling in the skip function.The URL handling logic could benefit from additional error handling:
- Validate URL format before opening
- Handle window.open failures
- Add logging for debugging purposes
export const skip = ({ api, router, fields }: Pick<INavMenuRenderlessParams, 'api' | 'router' | 'fields'>) => (item: menuItemType, flag = false): string | null => { if (!router) return if (item.isFullUrl) { const { urlField = 'url' } = fields || {} const router = item[urlField] || item.route - return (window.open(xss.filterUrl(router)).opener = null) + try { + const sanitizedUrl = xss.filterUrl(router) + const newWindow = window.open(sanitizedUrl) + if (newWindow) { + newWindow.opener = null + return sanitizedUrl + } + throw new Error('Failed to open window') + } catch (error) { + console.error('Failed to open URL:', error) + return null + } }packages/renderless/src/common/deps/useWindowSize.ts (1)
1-1: LGTM! Consider adding cleanup logic.The refactoring to use
@opentiny/utilsis done correctly, preserving the event listener options and maintaining the same functionality.Consider adding cleanup logic to remove event listeners when they're no longer needed:
export const useWindowSize = (ref) => () => { if (!width) { width = ref(0) height = ref(0) if (!dom.isServer) { const update = () => { width.value = window.innerWidth height.value = window.innerHeight } update() dom.on(window, 'resize', update, { passive: true }) dom.on(window, 'orientationchange', update, { passive: true }) + + return () => { + dom.off(window, 'resize', update) + dom.off(window, 'orientationchange', update) + } } } return { width, height } }Also applies to: 11-11, 19-20
packages/renderless/src/filter/vue.ts (1)
1-1: LGTM! Consider debouncing the resize handler.The migration to
@opentiny/utilsis done correctly. However, since both scroll and resize events can fire frequently, consider debouncing the resize handler for better performance.Example implementation:
import { dom } from '@opentiny/utils' // Add at the top level const RESIZE_DEBOUNCE_MS = 16 // ~1 frame at 60fps // In the renderless function const debouncedResize = () => { let timeoutId: number return (...args: any[]) => { clearTimeout(timeoutId) timeoutId = window.setTimeout(() => api.resize(...args), RESIZE_DEBOUNCE_MS) } } // In onMounted const handleResize = debouncedResize() dom.on(window, 'scroll', handleResize) dom.on(window, 'resize', handleResize) // In onBeforeUnmount dom.off(window, 'scroll', handleResize) dom.off(window, 'resize', handleResize)Also applies to: 63-64, 68-69
packages/vue/src/grid/src/menu/src/children.ts (1)
25-26: Consider consolidating utility imports.The code currently imports utilities from two different locations:
- getFuncText from @opentiny/vue-renderless/grid/utils
- dom from @opentiny/utils
Consider moving getFuncText to @opentiny/utils to maintain consistency with the PR's objective of consolidating utilities.
packages/vue/src/grid/src/menu/src/methods.ts (1)
Line range hint
13-15: Consider documenting the DOM utilities migration pattern.The consistent pattern of migrating from local DOM utilities to
@opentiny/utilsacross multiple packages suggests this is a significant architectural change. Consider:
- Documenting this pattern in the codebase's architectural guidelines
- Creating a migration guide for other packages that might need similar refactoring
- Adding JSDoc comments to the centralized
domutilities to help developers discover available methodsThis will help maintain consistency and guide future refactoring efforts.
Also applies to: 13-15, 15-15, 26-28
packages/renderless/src/dropdown/index.ts (1)
297-303: LGTM! Consistent use of DOM utilities for event cleanup inbeforeDistory.The change improves code consistency by using centralized DOM utilities for event cleanup.
Note: There's a typo in the function name
beforeDistory. It should bebeforeDestroy.-export const beforeDistory = +export const beforeDestroy =Also applies to: 310-311
packages/renderless/src/common/deps/ResizeObserver.ts (1)
252-252: Consider using optional chaining for mutation observer disconnection.To improve code safety, consider using optional chaining when disconnecting the mutation observer.
- this.mutationsObserver_ && this.mutationsObserver_.disconnect() + this.mutationsObserver_?.disconnect()🧰 Tools
🪛 Biome (1.9.4)
[error] 252-252: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue/src/grid/src/header/src/header.ts (1)
278-278: Consider using optional chaining.The current code can be improved using optional chaining for better null safety.
-let headOverflow = type.isNull(showHeaderOverflow) ? allColumnHeaderOverflow : showHeaderOverflow +let headOverflow = type.isNull(showHeaderOverflow) ? allColumnHeaderOverflow ?? showHeaderOverflow : showHeaderOverflow🧰 Tools
🪛 Biome (1.9.4)
[error] 278-278: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/common/deps/clickoutside.ts (1)
Line range hint
13-34: Well-structured refactoring with maintained functionality.The changes successfully migrate from local DOM utilities to the centralized
@opentiny/utilspackage while:
- Preserving the original click-outside detection logic
- Maintaining good documentation with clear usage examples
- Following the project's utility centralization goals
This refactoring improves maintainability without introducing breaking changes.
Consider adding JSDoc comments for the imported
domutility methods to improve IDE support and documentation coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
packages/renderless/src/amount/index.ts(3 hunks)packages/renderless/src/anchor/index.ts(3 hunks)packages/renderless/src/chart-core/index.ts(2 hunks)packages/renderless/src/common/deps/ResizeObserver.ts(4 hunks)packages/renderless/src/common/deps/clickoutside.ts(2 hunks)packages/renderless/src/common/deps/fullscreen/apis.ts(2 hunks)packages/renderless/src/common/deps/fullscreen/screenfull.ts(2 hunks)packages/renderless/src/common/deps/popper.ts(5 hunks)packages/renderless/src/common/deps/popup-manager.ts(7 hunks)packages/renderless/src/common/deps/repeat-click.ts(2 hunks)packages/renderless/src/common/deps/upload-ajax.ts(2 hunks)packages/renderless/src/common/deps/useEventListener.ts(4 hunks)packages/renderless/src/common/deps/useUserAgent.ts(1 hunks)packages/renderless/src/common/deps/useWindowSize.ts(2 hunks)packages/renderless/src/common/deps/vue-popper.ts(4 hunks)packages/renderless/src/common/deps/vue-popup.ts(2 hunks)packages/renderless/src/common/runtime.ts(0 hunks)packages/renderless/src/common/validate/util.ts(3 hunks)packages/renderless/src/crop/index.ts(2 hunks)packages/renderless/src/date-panel/index.ts(2 hunks)packages/renderless/src/dialog-box/index.ts(5 hunks)packages/renderless/src/drawer/index.ts(2 hunks)packages/renderless/src/dropdown-item/index.ts(2 hunks)packages/renderless/src/dropdown/index.ts(5 hunks)packages/renderless/src/fall-menu/index.ts(2 hunks)packages/renderless/src/file-upload/index.ts(6 hunks)packages/renderless/src/filter-panel/vue.ts(2 hunks)packages/renderless/src/filter/vue.ts(2 hunks)packages/renderless/src/floating-button/index.ts(3 hunks)packages/renderless/src/fluent-editor/index.ts(3 hunks)packages/renderless/src/fullscreen/index.ts(3 hunks)packages/renderless/src/grid/utils/dom.ts(4 hunks)packages/renderless/src/grid/utils/event.ts(2 hunks)packages/renderless/src/image-viewer/index.ts(5 hunks)packages/renderless/src/image/index.ts(4 hunks)packages/renderless/src/input/vue.ts(3 hunks)packages/renderless/src/load-list/index.ts(2 hunks)packages/renderless/src/load-list/vue.ts(2 hunks)packages/renderless/src/loading/index.ts(2 hunks)packages/renderless/src/modal/index.ts(13 hunks)packages/renderless/src/month-table/index.ts(2 hunks)packages/renderless/src/nav-menu/index.ts(5 hunks)packages/renderless/src/notify/index.ts(2 hunks)packages/renderless/src/numeric/index.ts(6 hunks)packages/renderless/src/picker-column/index.ts(3 hunks)packages/renderless/src/popconfirm/vue.ts(2 hunks)packages/renderless/src/popeditor/index.ts(6 hunks)packages/renderless/src/popover/index.ts(7 hunks)packages/renderless/src/popup/index.ts(3 hunks)packages/renderless/src/pull-refresh-mobile-first/index.ts(4 hunks)packages/renderless/src/pull-refresh-mobile-first/vue.ts(2 hunks)packages/renderless/src/pull-refresh/index.ts(2 hunks)packages/renderless/src/rate/vue.ts(2 hunks)packages/renderless/src/scrollbar/vue-bar.ts(2 hunks)packages/renderless/src/search/index.ts(4 hunks)packages/renderless/src/signature/index.ts(3 hunks)packages/renderless/src/slider-button-group/slide-button.ts(2 hunks)packages/renderless/src/slider/index.ts(5 hunks)packages/renderless/src/split/vue.ts(3 hunks)packages/renderless/src/steps/slide-bar.ts(2 hunks)packages/renderless/src/sticky/vue.ts(2 hunks)packages/renderless/src/tab-nav/index.ts(3 hunks)packages/renderless/src/tabbar/index.ts(2 hunks)packages/renderless/src/tabbar/vue.ts(2 hunks)packages/renderless/src/tall-storage/vue.ts(2 hunks)packages/renderless/src/tooltip/index.ts(4 hunks)packages/renderless/src/top-box/index.ts(2 hunks)packages/renderless/src/tree/index.ts(5 hunks)packages/utils/src/index.ts(1 hunks)packages/vue-common/package.json(1 hunks)packages/vue-common/src/breakpoint.ts(2 hunks)packages/vue/src/collapse-transition/package.json(1 hunks)packages/vue/src/collapse-transition/src/pc.vue(5 hunks)packages/vue/src/grid/src/composable/useDrag/dnd.ts(4 hunks)packages/vue/src/grid/src/edit/src/methods.ts(2 hunks)packages/vue/src/grid/src/header/src/header.ts(5 hunks)packages/vue/src/grid/src/keyboard/src/methods.ts(8 hunks)packages/vue/src/grid/src/keyboard/src/utils/triggerCellMousedownEvent.ts(2 hunks)packages/vue/src/grid/src/keyboard/src/utils/triggerHeaderCellMousedownEvent.ts(3 hunks)packages/vue/src/grid/src/menu/src/children.ts(2 hunks)packages/vue/src/grid/src/menu/src/methods.ts(2 hunks)packages/vue/src/grid/src/table/src/methods.ts(7 hunks)
⛔ Files not processed due to max files limit (7)
- packages/vue/src/grid/src/table/src/utils/handleFixedColumn.ts
- packages/vue/src/grid/src/tooltip/src/methods.ts
- packages/vue/src/loading/package.json
- packages/vue/src/loading/src/directive.ts
- packages/vue/src/loading/src/service.ts
- packages/vue/src/menu/package.json
- packages/vue/src/menu/src/menu-transition.vue
💤 Files with no reviewable changes (1)
- packages/renderless/src/common/runtime.ts
✅ Files skipped from review due to trivial changes (3)
- packages/renderless/src/grid/utils/event.ts
- packages/vue/src/grid/src/edit/src/methods.ts
- packages/renderless/src/rate/vue.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/renderless/src/search/index.ts
- packages/renderless/src/common/deps/upload-ajax.ts
- packages/renderless/src/amount/index.ts
- packages/renderless/src/common/validate/util.ts
- packages/renderless/src/popup/index.ts
- packages/renderless/src/image/index.ts
- packages/renderless/src/fluent-editor/index.ts
👮 Files not reviewed due to content moderation or server errors (4)
- packages/vue/src/collapse-transition/package.json
- packages/renderless/src/numeric/index.ts
- packages/renderless/src/modal/index.ts
- packages/renderless/src/common/deps/popper.ts
🧰 Additional context used
🪛 eslint
packages/renderless/src/load-list/index.ts
[error] 1-1: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
packages/vue/src/grid/src/keyboard/src/methods.ts
[error] 365-365: Replace ·dom.removeClass(elem,·'col__title-selected') with ⏎········dom.removeClass(elem,·'col__title-selected')⏎······
(prettier/prettier)
packages/renderless/src/common/deps/vue-popup.ts
[error] 15-15: 'dom' is defined but never used.
(unused-imports/no-unused-imports)
packages/renderless/src/common/deps/useEventListener.ts
[error] 1-1: Expected 1 empty line after import statement not followed by another import.
(import/newline-after-import)
packages/renderless/src/grid/utils/dom.ts
[error] 312-312: Insert ⏎
(prettier/prettier)
🪛 Biome (1.9.4)
packages/renderless/src/common/deps/ResizeObserver.ts
[error] 252-252: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/vue/src/grid/src/header/src/header.ts
[error] 278-278: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/renderless/src/numeric/index.ts
[error] 417-417: 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)
🔇 Additional comments (130)
packages/renderless/src/dropdown-item/index.ts (2)
19-19: LGTM! Import change aligns with centralization goals.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.
34-34: LGTM! DOM utility usage properly updated.The change to use
dom.on/dom.offmaintains the same functionality while making the DOM operation nature of these utilities more explicit.packages/renderless/src/scrollbar/vue-bar.ts (1)
22-22: LGTM! Import change aligns with refactoring goals.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.packages/renderless/src/tall-storage/vue.ts (2)
14-14: LGTM! Import consolidation aligns with PR objectives.The change to import the unified
domutility from@opentiny/utilsaligns with the PR's goal of centralizing utility functions.
38-38: LGTM! Consistent event handler updates.The event handler updates correctly use the new
domnamespace while maintaining the same functionality. The changes are properly applied to both the cleanup (off) and initialization (on) handlers.Also applies to: 41-41
packages/vue-common/src/breakpoint.ts (1)
3-3: LGTM! Import change aligns with centralization goals.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.packages/renderless/src/grid/utils/dom.ts (3)
27-27: LGTM! Import consolidation aligns with PR objectives.The change to import the unified
domutility from@opentiny/utilshelps centralize DOM manipulation functions.
196-196: LGTM! Proper usage of dom utility.The condition correctly uses both the imported
dom.hasClassand the localhasDataTagfunction.
241-241: LGTM! Correct usage of dom.getDomNode.The destructuring of
scrollTopandscrollLeftfromdom.getDomNode()is properly implemented.packages/renderless/src/pull-refresh/index.ts (3)
13-13: LGTM! Import change aligns with centralization goals.The change to import
domfrom@opentiny/utilsaligns well with the PR's objective of centralizing utility functions.
120-123: LGTM! Event binding changes are consistent.The event binding changes maintain the same functionality while using the centralized
domutility. All necessary touch events for pull-to-refresh are properly handled.
131-134: LGTM! Event cleanup is thorough and consistent.The event unbinding changes properly clean up all event listeners, preventing any potential memory leaks.
packages/renderless/src/slider/index.ts (3)
15-15: LGTM! Good practice using centralized DOM utilities.The import of
domfrom@opentiny/utilsaligns with the goal of centralizing utility functions, which will improve maintainability and consistency across the codebase.
20-20: LGTM! Consistent use of DOM event utilities.Event binding/unbinding has been updated to use the centralized
domutility while maintaining the same functionality. This change improves code consistency across the codebase.Also applies to: 24-24, 122-125, 179-182
108-113: LGTM! Consistent use of DOM class checking utilities.Class checking operations have been updated to use the centralized
dom.hasClassutility while maintaining the same functionality. This change improves code consistency across the codebase.Also applies to: 264-264
packages/renderless/src/image-viewer/index.ts (5)
15-15: LGTM! Import consolidation looks good.The change aligns with the PR objective of centralizing utilities into
@opentiny/utils.
50-50: LGTM! Event handling migration looks good.The event listeners have been consistently migrated to use the new
dom.onutility while preserving the existing logic and conditional checks.Also applies to: 129-130
136-137: LGTM! Event cleanup migration looks good.The event cleanup has been consistently migrated to use the new
dom.offutility while maintaining symmetry with the installation logic.
166-166: LGTM! Mouse event handling migration looks good.The mouse event handling has been consistently migrated to use the new
dom.on/offutilities while preserving the drag handling and cleanup logic.Also applies to: 168-168, 175-176, 179-180
788-788: LGTM! Cleanup migration looks good.The window resize event cleanup has been consistently migrated to use the new
dom.offutility while maintaining proper cleanup of all handlers and references.packages/renderless/src/load-list/vue.ts (2)
2-2: LGTM! Clean import of the dom utilities.The import change aligns with the PR objective of centralizing utility functions into @opentiny/utils.
71-71: LGTM! Correct usage of the namespaced utility.The function call has been properly updated to use the namespaced version (dom.getScrollParent) while maintaining the same functionality.
packages/renderless/src/pull-refresh-mobile-first/vue.ts (2)
33-33: LGTM! Clean import of the dom utilities.The import change aligns with the PR objective and is consistently placed with other imports.
129-129: LGTM! Correct usage of the namespaced utility.The function call has been properly updated to use the namespaced version (dom.getScrollParent) while maintaining the same functionality.
packages/renderless/src/common/deps/repeat-click.ts (1)
13-13: LGTM! Import change aligns with centralization goals.The change to import from
@opentiny/utilsinstead of local modules improves maintainability and follows the PR's objective of centralizing utility functions.packages/renderless/src/file-upload/index.ts (2)
35-35: LGTM! Import consolidation aligns with PR objectives.The change to import
typefrom@opentiny/utilsaligns with the PR's goal of centralizing utility functions.
62-62: LGTM! Consistent usage of type-checking utilities.The changes replace direct usage of
isObjectandisEmptyObjectwith their counterparts from thetypeutility module:
!isEmptyObject(props.hwh5)→!type.isEmptyObject(props.hwh5)isObject(file)→type.isObject(file)This refactoring maintains the same functionality while standardizing the use of type-checking utilities across the codebase.
Also applies to: 534-534, 2279-2279, 2453-2453, 2482-2482
packages/renderless/src/common/deps/useUserAgent.ts (3)
Line range hint
4-11: LGTM! Well-documented edge case handling.The updated server check maintains the same functionality while using the centralized utility. The implementation thoroughly handles browser detection, including the documented iPad Pro edge case.
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-7: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
13-16: LGTM! Clean and focused hook implementation.The hook follows Vue composition API best practices with a clear, single responsibility.
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-7: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
1-1: Verify the dom utility export from @opentiny/utilsThe import change aligns with the PR objective of centralizing utilities. Let's verify that the dom utility is properly exported.
✅ Verification successful
Import change is properly implemented and follows established patterns
The dom utility is correctly exported from @opentiny/utils and the import change follows the established pattern used across 70+ files in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify dom utility export and usage # Check if dom is properly exported from @opentiny/utils rg -g '!dist/*' -g '!node_modules/*' "export.*dom" "packages/utils/" # Check if other files have been updated to use the new import rg -g '!dist/*' -g '!node_modules/*' "import.*dom.*from.*@opentiny/utils"Length of output: 7063
packages/vue/src/grid/src/composable/useDrag/dnd.ts (2)
1-1: LGTM! Good refactoring of imports.The change to import the unified
domnamespace from@opentiny/utilsaligns well with the PR's objective of centralizing utility functions.
17-17: LGTM! Consistent use of dom utilities.The change to use
dom.preventDefaultmaintains the same behavior while aligning with the centralized utilities approach.packages/renderless/src/top-box/index.ts (2)
15-15: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.
34-34: Verify event binding behavior remains consistent.The change to
dom.on()looks correct, but let's verify that the event binding behavior remains consistent across the codebase.✅ Verification successful
Event binding implementation verified as consistent
The
dom.on(document, 'keydown', api.bindKeyDown)implementation matches the standard pattern used consistently across 32 other document event bindings in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of document event binding to ensure consistency rg "dom\.on\(document" --type tsLength of output: 3318
packages/renderless/src/popeditor/index.ts (3)
18-18: LGTM! Improved import organization.The change consolidates utility functions under
@opentiny/utils, which is a good practice for better maintainability.
97-102: LGTM! Consistent type checking.The changes standardize type checking by using the utility functions from
@opentiny/utils, making the code more consistent.
313-313: LGTM! Improved type conversion.The change ensures consistent type handling for numeric values, with proper NaN checking.
packages/renderless/src/common/deps/fullscreen/apis.ts (2)
14-14: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsfollows the PR's objective of centralizing utility functions.
131-131: LGTM! Event listener removal correctly uses new dom utility.The change maintains the same functionality while using the centralized utility.
packages/renderless/src/load-list/index.ts (1)
32-32: LGTM! Clean migration to centralized utility.The update to use
dom.isDisplayNonefrom@opentiny/utilsmaintains the same functionality while aligning with the centralized utilities approach.packages/renderless/src/signature/index.ts (2)
1-1: LGTM! Import change aligns with centralization objective.The change to import
domfrom@opentiny/utilsaligns with the PR's goal of centralizing utility functions.
1-1: Documentation needed for component usage and API.The PR checklist indicates documentation updates are missing. Given the complexity of this signature component, please add documentation covering:
- Component usage examples
- Available props and events
- Public methods for canvas manipulation
- Touch event handling behavior
Would you like me to help create a documentation template for this component?
packages/renderless/src/sticky/vue.ts (2)
42-42: LGTM! Function reference updated correctly.The update to use
dom.useScrollParentmaintains the same functionality while leveraging the centralized utility.
4-4: LGTM! Verify complete removal of old import.The addition of centralized
domutility import aligns with the PR objectives.Let's verify the complete removal of the old import:
✅ Verification successful
✅ Import migration verified successfully
The old import is completely removed and the new
domutility from@opentiny/utilsis properly used in the code, specifically for theuseScrollParentfunction.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any remaining references to the old import path rg "from '../common/deps/dom'" "packages/renderless/src/sticky/"Length of output: 70
Script:
#!/bin/bash # Get the complete file content cat packages/renderless/src/sticky/vue.ts # Search for any dom-related function calls in the file rg -l "useScrollParent|getScrollTop|getOffset" "packages/renderless/src/sticky/vue.ts"Length of output: 2294
packages/renderless/src/notify/index.ts (2)
14-14: LGTM! Import change aligns with centralization goal.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.
90-90: Verify event binding behavior remains consistent.The change to use
dom.onfrom@opentiny/utilsis appropriate. However, let's verify that the event binding behavior remains consistent across different browsers and scenarios.✅ Verification successful
Event binding implementation verified as consistent
The
dom.onusage for keydown events in the notify component follows the same established pattern used across 11 other components in the codebase, including similar global-scope components like modal and grid.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of dom.on for keydown events to ensure consistent implementation rg "dom\.on.*keydown" --type tsLength of output: 1183
packages/renderless/src/date-panel/index.ts (2)
33-33: LGTM! Import change aligns with PR objectives.The consolidation of DOM utilities under
@opentiny/utilsimproves code organization.
403-403: Verify event listener behavior after utility migration.The event listener implementation has been updated to use
dom.onanddom.off. While the change looks correct, please verify the keyboard navigation still works as expected.Run this script to verify other keyboard event handlers in the codebase:
Also applies to: 410-410
✅ Verification successful
Keyboard event listener implementation verified as correct.
The keyboard event handling implementation using
dom.on/offutilities follows the established patterns used successfully throughout the codebase for keyboard navigation and interaction.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of keyboard event handlers # Test: Look for other keyboard event handlers to ensure consistent implementation # Check for keyboard event handling patterns rg -A 5 "keydown|keyup|keypress" # Check for other uses of dom.on/off with keyboard events ast-grep --pattern 'dom.on($_, "key", $$$)' ast-grep --pattern 'dom.off($_, "key", $$$)'Length of output: 68110
packages/vue-common/package.json (2)
Line range hint
1-43: Package configuration looks good!The package configuration is well-structured with:
- Appropriate metadata (name, description, keywords)
- Correct version scheme
- Private flag set (as expected for internal packages)
- Proper workspace dependencies
31-31: Verify workspace dependency resolution.The dependency on
@opentiny/utilsuses the workspace protocol, which is correct for monorepo setups. Let's verify the workspace configuration:✅ Verification successful
Workspace dependency configuration is correct
The
workspace:~protocol is properly configured for this pnpm workspace, and the@opentiny/utilspackage exists in the workspace with matching version 1.0.0.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify workspace configuration and dependency resolution # Check if workspace is properly configured in root package.json echo "Checking workspace configuration..." jq '.workspaces' < package.json # Verify the package exists in the workspace echo "Verifying @opentiny/utils package..." fd -t f "package.json" -x jq -r 'select(.name=="@opentiny/utils") | .name, .version'Length of output: 349
Script:
#!/bin/bash # Check for package manager configuration echo "Checking for package manager configs..." ls pnpm-workspace.yaml yarn.workspace.* 2>/dev/null || echo "No workspace config found" # Find utils package location and content echo -e "\nLocating utils package..." fd -t f "package.json" -x sh -c 'echo "=== $1 ==="; cat "$1"' sh {} | grep -A 5 '"name": "@opentiny/utils"' # Check for package manager lock files echo -e "\nChecking lock files..." ls pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || echo "No lock files found"Length of output: 694
packages/renderless/src/floating-button/index.ts (1)
1-1: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilseffectively centralizes DOM utilities and reduces local utility duplications.packages/renderless/src/loading/index.ts (2)
15-15: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.
36-37: LGTM! Consistent usage of centralized DOM utilities.The changes correctly use the imported
dom.removeClassutility while maintaining the same functionality. The code properly handles class removal for both fullscreen and targeted loading cases.Let's verify that all
removeClasscalls in the codebase have been migrated:✅ Verification successful
Implementation verified as correct and consistent with package patterns
The usage of
dom.removeClassinpackages/renderless/src/loading/index.tsis correct and follows the established pattern in the renderless package. The codebase shows a clear separation where the mobile package intentionally uses directremoveClasscalls, while vue and renderless packages consistently use thedom.removeClassutility.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining direct removeClass calls that haven't been migrated # Search for direct removeClass calls rg "removeClass\(" --type ts --type vue -g "!node_modules" -g "!dist" # Search for the new pattern to confirm migration rg "dom\.removeClass\(" --type ts --type vue -g "!node_modules" -g "!dist"Length of output: 204
Script:
#!/bin/bash # Search for direct removeClass calls, excluding common build directories echo "=== Direct removeClass calls ===" rg "removeClass\(" --glob "!{node_modules,dist,build}/**" echo -e "\n=== dom.removeClass calls ===" rg "dom\.removeClass\(" --glob "!{node_modules,dist,build}/**"Length of output: 9749
packages/renderless/src/tree/index.ts (7)
18-18: LGTM! Import statement is correctly updated.The change from local DOM utility imports to
@opentiny/utilsis consistent with the PR objectives of centralizing utility functions.
139-139: LGTM! DOM manipulation is correctly updated.The change from direct
addClass/removeClasscalls todom.addClass/dom.removeClassmaintains the same functionality while using the centralized utilities.
194-194: LGTM! DOM manipulation in dragOver is correctly updated.The change to use
dom.removeClassis consistent with other DOM manipulation updates.
253-253: LGTM! DOM manipulation in dragEnd is correctly updated.The change to use
dom.removeClassmaintains consistency with other DOM manipulation updates.
398-398: LGTM! Event handling in mounted is correctly updated.The change from direct event binding to
dom.onis appropriate and consistent with the centralization of DOM utilities.
406-406: LGTM! Event handling in beforeUnmount is correctly updated.The change from direct event unbinding to
dom.offmaintains symmetry with the mounted hook's event binding.
Line range hint
1-824: Verify drag and drop functionality across browsers.The drag and drop implementation uses the HTML5 Drag and Drop API. Ensure consistent behavior across different browsers, especially for:
- Event.dataTransfer handling
- Drop effects
- Drag preview images
✅ Verification successful
Browser compatibility verified for drag and drop implementation.
The tree component's drag and drop implementation follows HTML5 standards and includes proper browser compatibility handling:
- Uses try-catch for dataTransfer.setData() browser differences
- Properly handles null dataTransfer objects
- Uses standard DOM APIs for positioning and visual feedback
- Includes defensive coding for edge cases
The implementation is robust and should work consistently across modern browsers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for browser-specific drag and drop handling rg -A 5 'dataTransfer|dragstart|dragend|dragover|drop'Length of output: 78616
packages/renderless/src/fall-menu/index.ts (2)
15-15: LGTM! Good refactoring of utility imports.The consolidation of utility functions into
@opentiny/utilsimproves code organization and maintainability.
119-119: LGTM! Good event handling practice.The use of
dom.onwith debouncedreRenderis a good practice for handling window resize events.packages/renderless/src/common/deps/popup-manager.ts (1)
14-14: LGTM! Good use of utility centralization.The import of DOM utilities from
@opentiny/utilsaligns with the PR's objective of centralizing utility functions.packages/renderless/src/slider-button-group/slide-button.ts (3)
2-2: LGTM! Import change aligns with centralization goal.The change to use
domfrom@opentiny/utilsis consistent with the PR's objective of centralizing utility functions.
156-156: LGTM! Proper cleanup of event listener.The change maintains proper cleanup while using the centralized utility.
161-161: LGTM! Verify event listener usage across the codebase.The change maintains proper event listener setup while using the centralized utility.
Let's verify the consistent usage of the new
dom.on/offpattern across the codebase:✅ Verification successful
Consistent with package-specific patterns
The change to use
dom.onis correct and follows the emerging pattern in the renderless package. While mobile packages use direct imports, renderless components are transitioning to thedom.on/offutility pattern. The implementation inslider-button.tsproperly follows this convention.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining direct imports of 'on/off' from local deps # and verify consistent usage of dom.on/dom.off # Check for any remaining direct imports rg "import.*\{.*(?:on|off).*\}.*from.*deps/dom" # Check for consistent usage of dom.on/dom.off ast-grep --pattern 'dom.on($$$)' --pattern 'dom.off($$$)'Length of output: 2094
Script:
#!/bin/bash # Check for new pattern usage echo "=== Checking dom.on usage ===" ast-grep --pattern 'dom.on($$$)' echo -e "\n=== Checking dom.off usage ===" ast-grep --pattern 'dom.off($$$)' echo -e "\n=== Checking direct on/off function calls ===" rg "\bon\s*\([^)]*window" -A 1 rg "\boff\s*\([^)]*window" -A 1Length of output: 38822
packages/renderless/src/month-table/index.ts (1)
14-14: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.packages/renderless/src/common/deps/useEventListener.ts (3)
17-17: LGTM! Good server-side handling.The early return using
dom.isServercheck prevents unnecessary event listener setup in server-side environments.
41-41: LGTM! Comprehensive cleanup implementation.The event listener cleanup is well-implemented with:
- Proper usage of
dom.off- Thorough cleanup across component lifecycle hooks
- Handling of dynamic target changes through watch
- Prevention of duplicate cleanup operations
Also applies to: 44-61
30-30: LGTM! Verify other usages of event listeners.The event listener attachment using
dom.onis implemented correctly with proper state management.Let's verify other usages of event listeners to ensure consistent implementation:
✅ Verification successful
Event listener implementation is consistent across codebase ✅
The change to use
dom.oninuseEventListener.tsfollows the established pattern used throughout the codebase for event listener management. All event listener attachments use the centralized DOM utilities for consistent behavior and proper cleanup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining direct usage of 'on' or 'off' functions # that should be using dom.on/dom.off instead rg --type ts "(?<!\.)(on|off)\s*\(\s*\w+\s*,\s*['\"]?\w+['\"]?\s*," -g '!node_modules'Length of output: 347
Script:
#!/bin/bash # Search for event listener related patterns with context rg --pcre2 --type ts -B 2 -A 2 '\bon\s*\(\s*\w+\s*,\s*[\'"]?\w+[\'"]?' -g '!node_modules' rg --pcre2 --type ts -B 2 -A 2 '\boff\s*\(\s*\w+\s*,\s*[\'"]?\w+[\'"]?' -g '!node_modules' # Also search for dom.on/dom.off usage for comparison rg --pcre2 --type ts -B 2 -A 2 'dom\.(on|off)\s*\(' -g '!node_modules'Length of output: 253
Script:
#!/bin/bash # Search for basic event listener patterns rg --type ts -B 2 -A 2 "\.on\(" -g '!node_modules' rg --type ts -B 2 -A 2 "\.off\(" -g '!node_modules' # Search for dom utility usage rg --type ts -B 2 -A 2 "dom\.on" -g '!node_modules' rg --type ts -B 2 -A 2 "dom\.off" -g '!node_modules'Length of output: 66846
packages/renderless/src/steps/slide-bar.ts (3)
2-2: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.
154-154: LGTM! Event listener properly implemented.The resize event listener is correctly implemented using the centralized
domutility and is appropriately placed in theonMountedhook.
158-158: LGTM! Event listener cleanup properly implemented.The resize event listener is correctly cleaned up using the centralized
domutility in theonUnmountedhook, preventing potential memory leaks.packages/renderless/src/nav-menu/index.ts (4)
24-24: LGTM! Import consolidation improves maintainability.The change to import utilities from
@opentiny/utilsaligns with the PR's goal of centralizing utility functions.
Line range hint
472-475: Good security practice using XSS filtering!The use of
xss.filterUrlwhen opening external URLs is a crucial security enhancement that helps prevent XSS attacks.
218-218: Ensure proper event cleanup with new DOM utilities.The migration to
dom.onanddom.offlooks good. However, let's verify that all event listeners are properly cleaned up:Also applies to: 240-240
✅ Verification successful
Event cleanup is properly implemented ✅
The window resize event is correctly registered with
dom.onand cleaned up withdom.offusing matching parameters.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining direct usage of on/off methods rg "(?<!dom\.)(on|off)\(window" --type ts # Search for paired usage of dom.on/dom.off rg "dom\.(on|off)\(window" --type tsLength of output: 5973
42-44: Verify type checking behavior across the codebase.The migration to
type.isEmptyObjectandtype.isObjectfrom the utils package should maintain consistent behavior. Let's verify other usages:Also applies to: 183-183
✅ Verification successful
Type checking changes are correctly implemented
The migration to
type.isEmptyObjectandtype.isObjectin nav-menu is consistent with the pattern used across the renderless components. However, there are some inconsistencies in other parts of the codebase where these utilities are used directly without proper imports.
internals/vite-plugin-template2jsx/src/render.js: Direct usage ofisEmptyObject- Multiple direct usages in mobile components and chart utilities
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining direct usage of isEmptyObject or isObject rg "(?<!type\.)(isEmptyObject|isObject)\(" --type ts # Search for new pattern usage for comparison rg "type\.(isEmptyObject|isObject)\(" --type tsLength of output: 3401
Script:
#!/bin/bash # Search for direct method calls using AST ast-grep --pattern 'isEmptyObject($_)' ast-grep --pattern 'isObject($_)' # Also search for imports of these methods rg "import.*\{.*(isEmptyObject|isObject).*\}" --type tsLength of output: 7836
packages/renderless/src/filter-panel/vue.ts (1)
2-2: LGTM! Clean refactoring of DOM utilities.The migration to
@opentiny/utilsis done correctly with proper event listener cleanup inonBeforeUnmount.Also applies to: 26-26, 30-30
packages/renderless/src/popconfirm/vue.ts (1)
7-7: LGTM! Well-structured refactoring with proper TypeScript support.The migration to
@opentiny/utilsmaintains type safety and correctly handles conditional event listener registration based oncloseOnClickOutside.Also applies to: 37-37, 40-40
packages/renderless/src/tabbar/index.ts (1)
13-13: LGTM! Clean refactoring of DOM utility import.The change correctly moves from local DOM utility to the centralized @opentiny/utils package, maintaining the same functionality.
Also applies to: 80-80
packages/renderless/src/tabbar/vue.ts (1)
14-14: LGTM! Consistent refactoring with companion file.The change maintains consistency with index.ts by using the same centralized DOM utility import.
Also applies to: 47-47
packages/vue/src/grid/src/keyboard/src/utils/triggerHeaderCellMousedownEvent.ts (1)
25-25: LGTM! Well-executed consolidation of DOM utilities.Multiple DOM utility imports have been cleanly consolidated into a single import from @opentiny/utils, with all usages correctly updated to use the dom namespace.
Also applies to: 41-41, 53-53, 57-57
packages/vue/src/collapse-transition/src/pc.vue (1)
20-20: LGTM! Improved code clarity by removing aliases.The refactoring successfully:
- Consolidates DOM utilities under @opentiny/utils
- Removes unnecessary aliases (addCls, rmvCls) in favor of the clearer dom namespace
- Maintains all transition functionality
Also applies to: 29-29, 57-57, 76-76, 85-85
packages/renderless/src/fullscreen/index.ts (1)
13-13: LGTM! Consistent usage of DOM utilities.The changes correctly migrate the event handling functions to use the consolidated DOM utilities from @opentiny/utils.
Also applies to: 38-39, 74-74
packages/renderless/src/common/deps/vue-popup.ts (1)
15-15: LGTM! Proper migration of DOM utilities.The changes correctly migrate the DOM utilities to use the consolidated utilities from @opentiny/utils.
Note: The eslint warning about unused 'dom' import is a false positive as dom.addClass is used in the code.
Also applies to: 125-125
🧰 Tools
🪛 eslint
[error] 15-15: 'dom' is defined but never used.
(unused-imports/no-unused-imports)
packages/renderless/src/common/deps/fullscreen/screenfull.ts (1)
14-14: LGTM! Consistent use of DOM utilities.The changes appropriately utilize the centralized DOM utilities from @opentiny/utils for event handling, maintaining the same functionality while improving code consistency.
Also applies to: 144-144, 151-151
packages/renderless/src/drawer/index.ts (1)
2-2: LGTM! Consistent use of DOM utilities for scroll management.The changes appropriately utilize the centralized DOM utilities for class manipulation, maintaining the same functionality while improving code consistency.
Also applies to: 172-172, 176-176
packages/vue/src/grid/src/keyboard/src/utils/triggerCellMousedownEvent.ts (1)
26-26: LGTM! Consistent use of DOM utilities for class checking.The changes appropriately utilize the centralized DOM utilities for class checking, maintaining the same functionality while improving code consistency.
Also applies to: 50-50
packages/renderless/src/crop/index.ts (1)
13-13: LGTM! Consistent use of DOM utilities for keyboard event handling.The changes appropriately utilize the centralized DOM utilities for event handling, maintaining the same functionality while improving code consistency.
Also applies to: 22-22, 24-24
packages/renderless/src/tooltip/index.ts (1)
141-146: LGTM! Consistent refactoring of DOM event handling.The changes successfully migrate event handling to use the centralized
domutilities from@opentiny/utils, maintaining the same functionality while improving code organization.Also applies to: 208-213
packages/renderless/src/picker-column/index.ts (1)
181-183: LGTM! Consistent refactoring of touch event handling.The changes successfully migrate touch event handling to use the centralized
domutilities from@opentiny/utils, maintaining the same functionality while improving code organization.Also applies to: 197-199
packages/renderless/src/common/deps/vue-popper.ts (1)
117-117: LGTM! Consistent refactoring of DOM utilities.The changes successfully migrate DOM visibility checks and event handling to use the centralized
domutilities from@opentiny/utils, maintaining the same functionality while improving code organization.Also applies to: 176-176, 220-220
packages/vue/src/grid/src/menu/src/methods.ts (1)
186-186: LGTM! Consistent refactoring of DOM node retrieval.The change successfully migrates DOM node retrieval to use the centralized
domutilities from@opentiny/utils, maintaining the same functionality while improving code organization.packages/renderless/src/anchor/index.ts (3)
13-13: LGTM! Import of DOM utilities from centralized package.The change aligns with the PR objective of centralizing utility functions.
33-35: LGTM! Consistent use of DOM utilities for class manipulation.The change improves code consistency by using centralized DOM utilities.
73-73: LGTM! Consistent use of DOM utilities for class manipulation.The change improves code consistency by using centralized DOM utilities.
packages/renderless/src/popover/index.ts (5)
15-15: LGTM! Import of DOM utilities from centralized package.The change aligns with the PR objective of centralizing utility functions.
26-27: LGTM! Consistent use of DOM utilities for event handling inprocessTrigger.The change improves code consistency by using centralized DOM utilities for event binding.
Also applies to: 29-32, 35-36, 38-39
68-68: LGTM! Consistent use of DOM utilities for class manipulation and event handling inmounted.The change improves code consistency by using centralized DOM utilities.
Also applies to: 76-89, 92-93
114-114: LGTM! Consistent use of DOM utilities for class manipulation in click and blur handlers.The change improves code consistency by using centralized DOM utilities.
Also applies to: 128-128, 134-134
239-246: LGTM! Consistent use of DOM utilities for event cleanup indestroyed.The change improves code consistency by using centralized DOM utilities for event cleanup.
Note: The comment "同步补充" (sync supplement) indicates additional event cleanup was added to ensure all event listeners are properly removed.
Also applies to: 249-255
packages/renderless/src/pull-refresh-mobile-first/index.ts (5)
13-13: LGTM! Import of DOM utilities from centralized package.The change aligns with the PR objective of centralizing utility functions.
78-80: LGTM! Consistent use of DOM utilities for event handling inmountedHandler.The change improves code consistency by using centralized DOM utilities for event binding.
88-90: LGTM! Consistent use of DOM utilities for event cleanup inbeforeUnmountHandler.The change improves code consistency by using centralized DOM utilities for event cleanup.
178-178: LGTM! Consistent use of DOM utilities for scroll position check.The change improves code consistency by using centralized DOM utilities.
208-208: LGTM! Consistent use of DOM utilities for event prevention.The change improves code consistency by using centralized DOM utilities.
packages/renderless/src/dropdown/index.ts (4)
15-15: LGTM! Import of DOM utilities from centralized package.The change aligns with the PR objective of centralizing utility functions.
37-37: LGTM! Consistent use of DOM utilities for class manipulation inwatchFocusing.The change improves code consistency by using centralized DOM utilities.
187-187: LGTM! Consistent use of DOM utilities for class manipulation ininitAria.The change improves code consistency by using centralized DOM utilities.
204-204: LGTM! Consistent use of DOM utilities for event handling ininitEvent.The change improves code consistency by using centralized DOM utilities for event binding.
Also applies to: 208-210, 218-221, 223-223
packages/renderless/src/dialog-box/index.ts (4)
14-14: LGTM! Import statement updated to use centralized utility.The change aligns with the PR objective of consolidating utility functions into the @opentiny/utils package.
95-95: LGTM! Event listeners updated to use centralized DOM utility.The scroll event handling has been properly migrated to use dom.on/off from @opentiny/utils.
Also applies to: 106-106
308-308: LGTM! DOM node retrieval updated to use centralized utility.The getDomNode function has been properly migrated to use dom.getDomNode from @opentiny/utils.
360-360: LGTM! Class manipulation updated to use centralized utility.The class manipulation functions have been properly migrated to use dom.addClass/removeClass from @opentiny/utils.
Also applies to: 364-364
packages/renderless/src/input/vue.ts (2)
59-59: LGTM! Import statement updated to use centralized utility.The change aligns with the PR objective of consolidating utility functions into the @opentiny/utils package.
410-410: LGTM! Event listeners updated to use centralized DOM utility.The resize event handling has been properly migrated to use dom.on/off from @opentiny/utils.
Also applies to: 420-420
packages/renderless/src/tab-nav/index.ts (1)
16-16: LGTM! Event handling updated to use centralized DOM utility.The event listeners for visibilitychange, blur, and focus events have been properly migrated to use dom.on/off from @opentiny/utils.
Also applies to: 128-130, 145-147
packages/renderless/src/common/deps/ResizeObserver.ts (2)
13-13: LGTM! Import statement updated to use centralized utility.The change aligns with the PR objective of consolidating utility functions into the @opentiny/utils package.
222-223: LGTM! Event handling updated to use centralized DOM utility.The event listeners for transitionend, resize, and DOMSubtreeModified events have been properly migrated to use dom.on/off from @opentiny/utils.
Also applies to: 237-237, 249-250, 255-255
packages/renderless/src/chart-core/index.ts (2)
23-23: LGTM! Import consolidation.The import of utility functions from
@opentiny/utilsaligns with the codebase's move towards centralized utilities.
438-438: LGTM! Event listener handling refactor.The update to use
dom.onanddom.offfor resize events maintains functionality while using the centralized DOM utility functions.Also applies to: 445-445
packages/vue/src/grid/src/header/src/header.ts (2)
168-168: LGTM! Type checking enhancement.The use of
type.isObjectimproves type safety and consistency in the codebase.
410-410: LGTM! DOM manipulation refactor.The update to use
dom.addClassanddom.removeClassmaintains functionality while using the centralized DOM utility functions.Also applies to: 544-544
packages/vue/src/grid/src/keyboard/src/methods.ts (1)
41-41: LGTM! Consistent DOM manipulation refactor.The update to use
dom.addClassanddom.removeClassacross multiple methods maintains functionality while using the centralized DOM utility functions consistently.Also applies to: 361-361, 365-365, 449-449, 521-521, 529-529, 542-542, 552-552, 621-621
packages/vue/src/grid/src/table/src/methods.ts (3)
381-381: LGTM! Type checking enhancement.The use of
type.isNullimproves type safety and consistency in the codebase.Also applies to: 499-499
983-983: LGTM! DOM visibility check refactor.The update to use
dom.isDisplayNonemaintains functionality while using the centralized DOM utility functions.
1187-1187: LGTM! DOM manipulation refactor.The update to use
dom.addClassanddom.removeClassacross multiple methods maintains functionality while using the centralized DOM utility functions consistently.Also applies to: 1199-1199, 1245-1245, 1251-1251
packages/renderless/src/common/deps/clickoutside.ts (3)
22-27: Verify mousedown event handling remains functional.The change from direct DOM event binding to using
dom.onmaintains the same functionality but through the centralized utility. The critical mousedown tracking logic is preserved.Let's verify there are no other mousedown handlers that might conflict:
✅ Verification successful
Mousedown event handling verified as safe.
The change to use
dom.onis safe as there are no conflicting document-level mousedown handlers. Other mousedown handlers in the codebase operate at different scopes (element-level) or serve as utility functions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other mousedown handlers in the codebase rg "mousedown.*=>" packages/renderless/src/Length of output: 313
Script:
#!/bin/bash # Check the implementation context of each mousedown handler echo "=== tall-storage/index.ts ===" rg -B 2 -A 2 "mousedown.*=>" packages/renderless/src/tall-storage/index.ts echo -e "\n=== repeat-click.ts ===" rg -B 2 -A 2 "mousedown.*=>" packages/renderless/src/common/deps/repeat-click.ts echo -e "\n=== clickoutside.ts ===" rg -B 2 -A 2 "mousedown.*=>" packages/renderless/src/common/deps/clickoutside.tsLength of output: 757
Line range hint
29-34: LGTM! Event handling with proper null checks.The mouseup event handler maintains the same logic while using the centralized
dom.onutility. The optional chaining (?.) provides good null-safety for the mouseupTrigger check.Let's verify the mouseup handler's uniqueness:
✅ Verification successful
No conflicts with other mouseup handlers ✅
The mouseup handler in clickoutside.ts uses
dom.on(addEventListener), while other handlers in modal and dialog-box use direct property assignment. These registration methods can safely coexist, and the other handlers are properly cleaned up after drag operations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other mouseup handlers that might interfere rg "mouseup.*=>" packages/renderless/src/Length of output: 366
Script:
#!/bin/bash # Get context for each mouseup handler echo "=== modal/index.ts ===" rg -B 5 -A 5 "mouseup.*=>" packages/renderless/src/modal/index.ts echo -e "\n=== dialog-box/index.ts ===" rg -B 5 -A 5 "mouseup.*=>" packages/renderless/src/dialog-box/index.ts echo -e "\n=== clickoutside.ts ===" rg -B 5 -A 5 "mouseup.*=>" packages/renderless/src/common/deps/clickoutside.tsLength of output: 1625
13-13: LGTM! Import change aligns with refactoring goals.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.Let's verify that the imported dom utility provides the same API:
✅ Verification successful
Import change is safe and compatible
The dom utility from @opentiny/utils provides identical
on()andoff()methods with the same API signature used in the clickoutside directive. The implementation is fully compatible with the existing usage.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the exported dom utility in @opentiny/utils rg -A 5 "export.*dom" packages/utils/Length of output: 298
Script:
#!/bin/bash # Find the dom utility implementation in @opentiny/utils fd dom.ts packages/utils/src -x cat {} # Find the original implementation or imports in clickoutside.ts cat packages/renderless/src/common/deps/clickoutside.tsLength of output: 3537
Script:
#!/bin/bash # Search for dom utility implementation ast-grep --pattern 'export const dom = $_' # Also search with ripgrep for any dom-related exports or implementations rg -g '!*.{json,md}' -A 5 "export.*dom" packages/Length of output: 4311
Script:
#!/bin/bash # Search for dom implementation in utils package fd -e ts -e js dom packages/utils/src -x cat {} # Also search for any file that might define dom methods rg -l "const dom = |function dom" packages/utils/Length of output: 103
Script:
#!/bin/bash # Search for files containing both 'on' and 'off' method implementations rg -l "\.on\s*=|\.off\s*=" packages/utils/ # Search for DOM-related utility files with different naming patterns fd -e ts -e js -E "*.d.ts" . packages/utils/src -x grep -l "addEventListener\|removeEventListener" {}Length of output: 182
Script:
#!/bin/bash # View the dom utility implementation cat packages/utils/src/dom/index.ts # Also check if there are any related test files that might show usage patterns fd -e spec.ts -e test.ts dom packages/utils/src -x cat {}Length of output: 7995
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/utils/src/type/index.ts (1)
Line range hint
159-162: Replace globalisNaNwithNumber.isNaNfor safer number comparison.The global
isNaNfunction attempts type coercion which can lead to unexpected results. UseNumber.isNaNinstead for more reliable NaN checking.export const isSame = (x: any, y: any) => - x === y || (typeof x === 'number' && typeof y === 'number' && isNaN(x) && isNaN(y)) + x === y || (typeof x === 'number' && typeof y === 'number' && Number.isNaN(x) && Number.isNaN(y))
🧹 Nitpick comments (9)
packages/renderless/src/common/deps/popper.ts (3)
784-787: Consider using optional chaining for DOM check.The DOM visibility check could be simplified using optional chaining.
-if (dom.isDisplayNone(this._reference)) return +if (dom.isDisplayNone(this._reference ?? null)) return
Line range hint
874-880: LGTM! Global config access is properly updated.The change correctly uses optional chaining to safely access the viewport window from the global config.
However, consider adding a JSDoc comment to explain the fallback chain:
+/** Fallback chain for viewport window: + * 1. global.globalConfig?.viewportWindow + * 2. PopupManager.viewportWindow + * 3. window + */ const viewportWindow = global.globalConfig?.viewportWindow || PopupManager.viewportWindow || window
Line range hint
1-900: Consider adding comprehensive JSDoc documentation.The codebase would benefit from more detailed JSDoc documentation for the Popper class and its methods, especially since it's a core positioning utility.
Example for the class:
/** * Popper class handles the positioning of floating elements in relation to reference elements. * It supports various placement options, modifiers, and boundary constraints. * * @class Popper * @example * const popper = new Popper(referenceElement, popperElement, { * placement: 'bottom', * offset: 10 * }); */packages/utils/src/type/index.ts (3)
150-150: Consider using more specific types instead ofany.Using
anytype annotations reduces TypeScript's type safety benefits. Consider:
- Using
unknownforisDatesince we're type-checking the value- Using either
unknownor a generic type with constraints forisPromise-export const isDate = (value: any) => typeOf(value) === 'date' +export const isDate = (value: unknown) => typeOf(value) === 'date' -export const isPromise = (val: any) => isObject(val) && isFunction(val.then) && isFunction(val.catch) +export const isPromise = <T = unknown>(val: unknown): val is Promise<T> => + isObject(val) && isFunction((val as any).then) && isFunction((val as any).catch)Also applies to: 166-166
168-183: Consider using only named exports for better maintainability.Having both named exports and a default export can lead to inconsistent import patterns across the codebase. Since all functions are already available as named exports, consider removing the default export to encourage consistent usage of named imports.
-export default { - toString, - hasOwn, - isNull, - typeOf, - isObject, - isFunction, - isPlainObject, - isEmptyObject, - isNumber, - isNumeric, - isDate, - isSame, - isRegExp, - isPromise -}🧰 Tools
🪛 Biome (1.9.4)
[error] 181-182: 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)
150-150: Enhance TypeScript documentation for better developer experience.Consider adding TypeScript-specific JSDoc documentation for the modified functions:
- Add
@paramand@returnstags with type information- Include TypeScript-specific examples in the documentation
Example for
isPromise:/** * Checks if a value is a Promise-like object. * @param val - The value to check * @returns True if the value is a Promise-like object * @example * ```typescript * isPromise(Promise.resolve(42)) // true * isPromise({ then: () => {}, catch: () => {} }) // true * isPromise(42) // false * ``` */Also applies to: 166-166
packages/renderless/src/scrollbar/vue-bar.ts (1)
50-52: LGTM! API configuration properly updated.The API object extensions have been correctly updated to use the new
domutilities. However, since the PR checklist indicates documentation updates are missing, consider adding JSDoc comments to document these changes.Consider adding documentation:
return Object.assign(api, { + /** Initiates drag operation using centralized dom.on utility */ startDrag: startDrag({ api, on: dom.on, state }), clickThumbHandler: clickThumbHandler({ api, state }), + /** Handles mouseup events using centralized dom.off utility */ mouseUpDocumentHandler: mouseUpDocumentHandler({ api, off: dom.off, state }) })packages/utils/src/dom/index.ts (2)
Line range hint
31-35: Enhance type safety with proper TypeScript types.Several functions could benefit from stronger type definitions:
- The
eventparameter inon/offfunctions usesanytypegetScrollTopandpreventDefaultparameters lack type annotations- Return types are not explicitly defined
Consider applying these type improvements:
-export const on = (el: EventTarget, event: any, handler: (this: HTMLElement, ev: any) => any, options = false) => { +export const on = <K extends keyof HTMLElementEventMap>( + el: EventTarget, + event: K, + handler: (this: HTMLElement, ev: HTMLElementEventMap[K]) => any, + options: boolean | AddEventListenerOptions = false +): void => { -export const getScrollTop = (el) => { +export const getScrollTop = (el: Element | Window): number => { -export const preventDefault = (event, isStopPropagation) => { +export const preventDefault = (event: Event, isStopPropagation?: boolean): void => {Also applies to: 71-75, 249-253
🧰 Tools
🪛 Biome (1.9.4)
[error] 305-312: 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)
309-327: LGTM! Consider adding usage examples in documentation.The default export effectively aggregates all DOM utilities, allowing for both named and default imports. This aligns well with the PR objective of centralizing utility functions.
Consider adding JSDoc comments at the top of the file to demonstrate both import patterns:
/** * DOM utility functions for TinyVue * * @example * // Import all utilities * import dom from '@opentiny/utils/dom' * dom.addClass(element, 'class-name') * * // Import specific utilities * import { addClass, removeClass } from '@opentiny/utils/dom' * addClass(element, 'class-name') */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/renderless/src/amount/index.ts(3 hunks)packages/renderless/src/cascader/index.ts(4 hunks)packages/renderless/src/common/deps/popper.ts(5 hunks)packages/renderless/src/common/deps/vue-popper.ts(4 hunks)packages/renderless/src/date-panel/index.ts(2 hunks)packages/renderless/src/dropdown/index.ts(5 hunks)packages/renderless/src/image/index.ts(4 hunks)packages/renderless/src/modal/index.ts(13 hunks)packages/renderless/src/picker/index.ts(7 hunks)packages/renderless/src/scrollbar/vue-bar.ts(2 hunks)packages/renderless/src/split/vue.ts(3 hunks)packages/utils/src/dom/index.ts(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/src/type/index.ts(2 hunks)packages/vue/src/image-viewer/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/renderless/src/amount/index.ts
- packages/renderless/src/date-panel/index.ts
- packages/renderless/src/dropdown/index.ts
- packages/renderless/src/modal/index.ts
- packages/utils/src/index.ts
- packages/renderless/src/cascader/index.ts
- packages/renderless/src/common/deps/vue-popper.ts
- packages/renderless/src/split/vue.ts
- packages/renderless/src/image/index.ts
- packages/renderless/src/picker/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/utils/src/type/index.ts
[error] 181-182: 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 (7)
packages/vue/src/image-viewer/package.json (1)
18-18: LGTM! The dependency addition aligns with the PR objectives.The addition of
@opentiny/utilsas a workspace dependency is consistent with:
- The PR's goal of centralizing utility functions
- The versioning pattern used for other workspace dependencies
packages/renderless/src/common/deps/popper.ts (3)
14-14: LGTM! Import consolidation looks good.The change consolidates utility imports from individual modules into a single import from
@opentiny/utils, which aligns with the PR objective of centralizing utility functions.
453-453: LGTM! Type checking update is correct.The change from direct
typeOfusage totype.typeOfmaintains the same functionality while using the new centralized utilities.
789-789: LGTM! DOM event handling updates are consistent.The changes consistently update all DOM event handling to use the new
domutility methods from@opentiny/utils.Also applies to: 817-817, 823-823, 826-826, 833-833, 836-836, 844-844
packages/renderless/src/scrollbar/vue-bar.ts (2)
47-47: LGTM! Event cleanup properly updated.The event cleanup in
onUnmountedhas been correctly updated to use the newdom.offutility while maintaining the same cleanup functionality.
22-22: LGTM! Import change aligns with centralization effort.The change to import
domfrom@opentiny/utilsaligns with the PR's objective of centralizing utility functions.Let's verify the consistency of this change across the codebase:
✅ Verification successful
Import change verified and consistent within scrollbar component
The migration to
import { dom } from '@opentiny/utils'is correctly implemented in the scrollbar component. While some files in the codebase (primarily mobile components) still use the old import path, the scrollbar component's implementation is consistent with the new pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any remaining direct imports of on/off from the old path rg "from '\.\./common/deps/dom'" packages/Length of output: 50
Script:
#!/bin/bash # Check for any remaining imports from deps/dom with a more flexible pattern rg -l "deps/dom" # Check current usage of dom from @opentiny/utils rg "import.*dom.*@opentiny/utils" # Look specifically in scrollbar-related files fd scrollbar -t f -e ts -e vue -x rg -l "deps/dom" {}Length of output: 8113
packages/utils/src/dom/index.ts (1)
Line range hint
13-15: Consider consolidating type-related imports with the PR objective.Since this PR aims to extract functions from the type module to utils, consider moving the
hasOwnandisNullfunctions to a dedicated utils file if they haven't been moved already.Let's verify the current location of these type utilities:
🧰 Tools
🪛 Biome (1.9.4)
[error] 305-312: 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)
wuyiping0628
changed the title
shenjunjian
changed the title
wuyiping0628
changed the title
PR
此次提交把dom.gloabl.type三个文件下面的公共方法提取到untils包下面，其他引用的地方直接引用这个包就行，目前本地测试用例已经通过
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
Release Notes
New Features
@opentiny/utilsas a centralized utility library across multiple packagesImprovements
@opentiny/utilspackageChanges
@opentiny/utilsCompatibility