-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ballerine Validator(WIP) #2874
base: dev
Are you sure you want to change the base?
Ballerine Validator(WIP) #2874
Changes from 2 commits
75d4a34
b2c7a44
e471f81
7cb51fe
4d2c708
c2987da
ce18a0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
./_Validator |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import { ValidatorContext } from './context'; | ||
import { IValidatorRef, useValidatorRef } from './hooks/internal/useValidatorRef'; | ||
import { IValidationSchema } from './types'; | ||
|
||
export interface IValidatorProviderProps<TValue> { | ||
children: React.ReactNode | React.ReactNode[]; | ||
schema: IValidationSchema[]; | ||
value: TValue; | ||
|
||
ref?: React.RefObject<IValidatorRef>; | ||
validateOnChange?: boolean; | ||
} | ||
|
||
export const ValidatorProvider = <TValue,>({ | ||
children, | ||
schema, | ||
value, | ||
ref, | ||
}: IValidatorProviderProps<TValue>) => { | ||
useValidatorRef(ref); | ||
|
||
return <ValidatorContext.Provider value={{}}>{children}</ValidatorContext.Provider>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement proper context value The context provider is initialized with an empty object, which doesn't match the expected interface from the AI summary. Implement the required context value structure: - return <ValidatorContext.Provider value={{}}>{children}</ValidatorContext.Provider>;
+ return (
+ <ValidatorContext.Provider
+ value={{
+ errors: [],
+ values: value,
+ validate: async () => {
+ // Implement validation logic
+ },
+ }}
+ >
+ {children}
+ </ValidatorContext.Provider>
+ );
|
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './types'; | ||
export * from './validator-context'; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
import { IValidationError } from '../types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
export interface IValidatorContext<TValues> { | ||||||||||||||||||||||||||||||||||||||||||||||||||
errors: IValidationError[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
values: TValues; | ||||||||||||||||||||||||||||||||||||||||||||||||||
isValid: boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||||
validate: () => void; | ||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance type definitions for better validation control The current interface could be improved to provide better type safety and flexibility: Consider these enhancements: +/** Options for validation execution */
+export interface IValidationOptions {
+ validateAll?: boolean;
+ stopOnFirst?: boolean;
+}
+
+/** Result of validation execution */
+export interface IValidationResult {
+ isValid: boolean;
+ errors: IValidationError[];
+}
export interface IValidatorContext<TValues> {
errors: IValidationError[];
values: TValues;
isValid: boolean;
- validate: () => void;
+ validate: (options?: IValidationOptions) => Promise<IValidationResult>;
} This provides:
Consider adding JSDoc comments to document the interface and its generic parameter usage. 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { createContext } from 'react'; | ||
import { IValidatorContext } from './types'; | ||
|
||
export const ValidatorContext = createContext<IValidatorContext<unknown>>({ | ||
errors: [], | ||
values: {}, | ||
isValid: true, | ||
validate: () => { | ||
throw new Error('Validator context is not provided.'); | ||
}, | ||
}); | ||
Comment on lines
+4
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider using a more specific generic type parameter The context is currently typed with -export const ValidatorContext = createContext<IValidatorContext<unknown>>({
+export const ValidatorContext = createContext<IValidatorContext<TValue extends object>>({ Also, the error message could be more descriptive about where to add the provider: validate: () => {
- throw new Error('Validator context is not provided.');
+ throw new Error('ValidatorContext.Provider is not found in component tree. Wrap your component with ValidatorProvider.');
},
Comment on lines
+1
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Confirmed: Two separate validator contexts exist with overlapping functionality There are indeed two separate validator contexts with similar purposes but different implementations:
Both contexts handle form validation but have different interfaces and are used in separate components. This creates technical debt and potential confusion:
Recommendations:
🔗 Analysis chainVerify potential context naming conflicts There appears to be two validator contexts in the codebase ( Let me check the actual content of these files to understand if there's a real conflict or if these are related components. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for validator context definitions and usages
rg -l "validatorContext|ValidatorContext" --type ts
Length of output: 602 Script: #!/bin/bash
# Check the content of both validator context files and their usages
echo "=== validator-context.ts ==="
cat packages/ui/src/components/organisms/Form/Validator/context/validator-context.ts
echo -e "\n=== validator.context.ts ==="
cat packages/ui/src/components/organisms/Form/_Validator/validator.context.ts
echo -e "\n=== Checking imports in _Validator/Validator.tsx ==="
rg "import.*[Vv]alidator[Cc]ontext" packages/ui/src/components/organisms/Form/_Validator/Validator.tsx
echo -e "\n=== Checking imports in Validator/ValidatorProvider.tsx ==="
rg "import.*[Vv]alidator[Cc]ontext" packages/ui/src/components/organisms/Form/Validator/ValidatorProvider.tsx
Length of output: 1534 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './useValidator'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { useContext } from 'react'; | ||
import { ValidatorContext } from '../../../context'; | ||
|
||
export const useValidator = () => { | ||
const context = useContext(ValidatorContext); | ||
|
||
if (!context) { | ||
throw new Error('Validator context is not provided.'); | ||
} | ||
|
||
return context; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './types'; | ||
export * from './useValidatorRef'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
export interface IValidatorRef { | ||
validate: () => void; | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useImperativeHandle } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useValidator } from '../../external/useValidator/useValidator'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { IValidatorRef } from './types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const useValidatorRef = (refObject?: React.RefObject<IValidatorRef>): IValidatorRef => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const context = useValidator(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useImperativeHandle(refObject, () => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
validate: context.validate, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return context; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+5
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling and optimize performance The current implementation could benefit from error handling and performance optimizations: Consider these improvements: export const useValidatorRef = (refObject?: React.RefObject<IValidatorRef>): IValidatorRef => {
const context = useValidator();
+ if (!context) {
+ throw new Error('useValidatorRef must be used within a ValidatorProvider');
+ }
+
+ const exposedRef = useMemo(
+ () => ({
+ validate: context.validate,
+ }),
+ [context.validate]
+ );
+
- useImperativeHandle(refObject, () => ({
- validate: context.validate,
- }));
+ useImperativeHandle(refObject, () => exposedRef, [exposedRef]);
return context;
}; Consider adding error boundaries in the parent component to gracefully handle validation errors and provide meaningful feedback to users. 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,45 @@ | ||||||||||||||||||||||||||||
import { renderHook } from '@testing-library/react'; | ||||||||||||||||||||||||||||
import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||||||||||||||||||||||||||||
import { useValidator } from '../../external/useValidator/useValidator'; | ||||||||||||||||||||||||||||
import { useValidatorRef } from './useValidatorRef'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
const mockValidate = vi.fn(); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
vi.mock('../../external/useValidator/useValidator', () => ({ | ||||||||||||||||||||||||||||
useValidator: vi.fn(() => ({ | ||||||||||||||||||||||||||||
validate: mockValidate, | ||||||||||||||||||||||||||||
})), | ||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve mock implementation robustness The current mock implementation could be enhanced to better test error scenarios. vi.mock('../../external/useValidator/useValidator', () => ({
useValidator: vi.fn(() => ({
validate: mockValidate,
+ isValid: true,
+ errors: [],
+ validateField: vi.fn(),
})),
})); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
describe('useValidatorRef', () => { | ||||||||||||||||||||||||||||
const mockRef = { current: null }; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
beforeEach(() => { | ||||||||||||||||||||||||||||
vi.clearAllMocks(); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it('should return context from useValidator', () => { | ||||||||||||||||||||||||||||
const { result } = renderHook(() => useValidatorRef()); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
expect(result.current).toEqual({ | ||||||||||||||||||||||||||||
validate: mockValidate, | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
expect(useValidator).toHaveBeenCalled(); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it('should set ref.current.validate to context.validate when ref is provided', () => { | ||||||||||||||||||||||||||||
renderHook(() => useValidatorRef(mockRef)); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
expect(mockRef.current).toEqual({ | ||||||||||||||||||||||||||||
validate: mockValidate, | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
it('should not set ref when no ref object is provided', () => { | ||||||||||||||||||||||||||||
const { result } = renderHook(() => useValidatorRef()); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
expect(result.current).toEqual({ | ||||||||||||||||||||||||||||
validate: mockValidate, | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
Comment on lines
+14
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add test coverage for error scenarios The test suite is missing coverage for error handling scenarios. Add these test cases: it('should handle errors during validation', () => {
mockValidate.mockImplementationOnce(() => {
throw new Error('Validation failed');
});
const { result } = renderHook(() => useValidatorRef(mockRef));
expect(() => result.current.validate()).toThrow('Validation failed');
});
it('should handle undefined ref gracefully', () => {
const { result } = renderHook(() => useValidatorRef(undefined));
expect(result.current.validate).toBeDefined();
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export * from './hooks/external/useValidator'; | ||
export * from './types'; | ||
export * from './utils/register-validator'; | ||
export * from './ValidatorProvider'; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,44 @@ | ||||||||||||||||||||||||||||||||||||
export type TBaseValidationRules = 'json-logic'; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export interface IValidationRule { | ||||||||||||||||||||||||||||||||||||
type: TBaseValidationRules; | ||||||||||||||||||||||||||||||||||||
value: object; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety and documentation for validation rules
-export type TBaseValidationRules = 'json-logic';
+/** Supported validation rule types */
+export type TBaseValidationRules =
+ | 'json-logic'
+ | 'expression' // Future rule types
+ | 'custom'; // Future rule types
+/** Structure of a validation rule */
export interface IValidationRule {
type: TBaseValidationRules;
- value: object;
+ value: Record<string, unknown>;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export type TBaseValidators = | ||||||||||||||||||||||||||||||||||||
| 'required' | ||||||||||||||||||||||||||||||||||||
| 'minLength' | ||||||||||||||||||||||||||||||||||||
| 'maxLength' | ||||||||||||||||||||||||||||||||||||
| 'pattern' | ||||||||||||||||||||||||||||||||||||
| 'minimum' | ||||||||||||||||||||||||||||||||||||
| 'maximum'; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export interface ICommonValidator<T = object, TValidatorType = TBaseValidators> { | ||||||||||||||||||||||||||||||||||||
type: TValidatorType; | ||||||||||||||||||||||||||||||||||||
value: T; | ||||||||||||||||||||||||||||||||||||
message?: string; | ||||||||||||||||||||||||||||||||||||
applyWhen?: IValidationRule; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export interface IValidationSchema { | ||||||||||||||||||||||||||||||||||||
id: string; | ||||||||||||||||||||||||||||||||||||
valueDestination?: string; | ||||||||||||||||||||||||||||||||||||
validators: ICommonValidator[]; | ||||||||||||||||||||||||||||||||||||
children?: IValidationSchema[]; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export interface IValidationError { | ||||||||||||||||||||||||||||||||||||
id: string; | ||||||||||||||||||||||||||||||||||||
originId: string; | ||||||||||||||||||||||||||||||||||||
invalidValue: unknown; | ||||||||||||||||||||||||||||||||||||
message: string[]; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+33
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Based on the search results, I can now generate the final response as we have found two different implementations of Validation error message type inconsistency needs to be resolved There are two different implementations of
This inconsistency needs to be resolved to maintain type safety and prevent potential runtime errors. 🔗 Analysis chainReview message type consistency across validation system There's a potential inconsistency in the message handling:
Let's verify the message handling across the validation system: Let me search for the actual error handling implementation to see how validation errors are collected and processed. Let me try a different approach to find how validation errors are collected and processed. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for message handling in validators and error processing
rg -A 2 "message[\"']?:" packages/ui/src/components/organisms/Form/
Length of output: 30214 Script: #!/bin/bash
# Search for validation error handling implementation
ast-grep --pattern 'catch ($_) {
$$$
}'
Length of output: 43 Script: #!/bin/bash
# Search for validation error handling and collection
rg -A 5 "IValidationError" packages/ui/src/components/organisms/Form/
Length of output: 4835 |
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export * from '../hooks/internal/useValidatorRef/types'; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export type TValidator<T, TValidatorParams = unknown> = ( | ||||||||||||||||||||||||||||||||||||
value: T, | ||||||||||||||||||||||||||||||||||||
validator: ICommonValidator<TValidatorParams>, | ||||||||||||||||||||||||||||||||||||
) => void; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export type TDeepthLevelStack = number[]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { TDeepthLevelStack } from '../../types'; | ||
|
||
import { IValidationError } from '../../types'; | ||
import { formatId } from '../format-id'; | ||
|
||
export const createValidationError = ({ | ||
id, | ||
invalidValue, | ||
message, | ||
stack, | ||
}: { | ||
id: string; | ||
invalidValue: unknown; | ||
message: string; | ||
stack: TDeepthLevelStack; | ||
}): IValidationError => { | ||
const formattedId = formatId(id, stack); | ||
|
||
const error: IValidationError = { | ||
id: formattedId, | ||
originId: id, | ||
invalidValue, | ||
message: [message], | ||
}; | ||
|
||
return error; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import { describe, expect, it } from 'vitest'; | ||
import { createValidationError } from './create-validation-error'; | ||
|
||
describe('createValidationError', () => { | ||
it('should create validation error with formatted id', () => { | ||
const params = { | ||
id: 'test', | ||
invalidValue: 'invalid', | ||
message: 'error message', | ||
stack: [1, 2], | ||
}; | ||
|
||
const result = createValidationError(params); | ||
|
||
expect(result).toEqual({ | ||
id: 'test-1-2', | ||
originId: 'test', | ||
invalidValue: 'invalid', | ||
message: ['error message'], | ||
}); | ||
}); | ||
|
||
it('should handle empty stack', () => { | ||
const params = { | ||
id: 'test', | ||
invalidValue: 123, | ||
message: 'error message', | ||
stack: [], | ||
}; | ||
|
||
const result = createValidationError(params); | ||
|
||
expect(result).toEqual({ | ||
id: 'test-', | ||
originId: 'test', | ||
invalidValue: 123, | ||
message: ['error message'], | ||
}); | ||
}); | ||
|
||
it('should handle single stack value', () => { | ||
const params = { | ||
id: 'test', | ||
invalidValue: null, | ||
message: 'error message', | ||
stack: [1], | ||
}; | ||
|
||
const result = createValidationError(params); | ||
|
||
expect(result).toEqual({ | ||
id: 'test-1', | ||
originId: 'test', | ||
invalidValue: null, | ||
message: ['error message'], | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './create-validation-error'; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,2 @@ | ||||||||||||||||||||||||
export const formatErrorMessage = (message: string, key: string, value: string) => | ||||||||||||||||||||||||
message.replaceAll(`{${key}}`, value); | ||||||||||||||||||||||||
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add input validation and improve type safety The current implementation could be enhanced in several ways:
Consider this improved implementation: -export const formatErrorMessage = (message: string, key: string, value: string) =>
- message.replaceAll(`{${key}}`, value);
+export const formatErrorMessage = (
+ message: string,
+ key: string,
+ value: string | number | boolean
+) => {
+ if (!message || !key) return message;
+ const safeValue = String(value);
+ return message.replaceAll(`{${key}}`, safeValue);
+}; 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { describe, expect, it } from 'vitest'; | ||
import { formatErrorMessage } from './format-error-message'; | ||
|
||
describe('formatErrorMessage', () => { | ||
it('should replace single placeholder with value', () => { | ||
const message = 'This is a {test} message'; | ||
const result = formatErrorMessage(message, 'test', 'sample'); | ||
expect(result).toBe('This is a sample message'); | ||
}); | ||
|
||
it('should replace multiple occurrences of the same placeholder', () => { | ||
const message = 'The {value} is equal to {value}'; | ||
const result = formatErrorMessage(message, 'value', '42'); | ||
expect(result).toBe('The 42 is equal to 42'); | ||
}); | ||
|
||
it('should not modify message when placeholder is not found', () => { | ||
const message = 'This message has no placeholders'; | ||
const result = formatErrorMessage(message, 'key', 'value'); | ||
expect(result).toBe('This message has no placeholders'); | ||
}); | ||
|
||
it('should handle empty strings', () => { | ||
const message = ''; | ||
const result = formatErrorMessage(message, 'key', 'value'); | ||
expect(result).toBe(''); | ||
}); | ||
|
||
it('should handle special characters in placeholder values', () => { | ||
const message = 'Special {char} test'; | ||
const result = formatErrorMessage(message, 'char', '$@#'); | ||
expect(result).toBe('Special $@# test'); | ||
}); | ||
}); | ||
Comment on lines
+4
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance test coverage for edge cases and security The test suite should include additional scenarios for better coverage and security validation. Add these test cases: it('should handle nested placeholders', () => {
const message = 'Nested {outer{inner}} test';
const result = formatErrorMessage(message, 'outer{inner}', 'value');
expect(result).toBe('Nested value test');
});
it('should escape HTML in values to prevent XSS', () => {
const message = 'Test {script}';
const result = formatErrorMessage(message, 'script', '<script>alert("xss")</script>');
expect(result).toBe('Test <script>alert("xss")</script>');
});
it('should handle malformed placeholders', () => {
const message = 'Malformed {key test';
const result = formatErrorMessage(message, 'key', 'value');
expect(result).toBe('Malformed {key test');
});
it('should handle multiple different placeholders', () => {
const message = '{first} and {second}';
const result1 = formatErrorMessage(message, 'first', '1');
const result2 = formatErrorMessage(result1, 'second', '2');
expect(result2).toBe('1 and 2');
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './format-error-message'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { TDeepthLevelStack } from '../../types'; | ||
|
||
export const formatId = (id: string, stack: TDeepthLevelStack) => { | ||
const _id = `${id}${stack.length ? `-${stack.join('-')}` : ''}`; | ||
|
||
return _id; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { describe, expect, it } from 'vitest'; | ||
import { formatId } from './format-id'; | ||
|
||
describe('formatId', () => { | ||
it('should append stack values to id', () => { | ||
const id = 'test'; | ||
const stack = [1, 2]; | ||
|
||
const result = formatId(id, stack); | ||
|
||
expect(result).toBe('test-1-2'); | ||
}); | ||
|
||
it('should handle empty stack', () => { | ||
const id = 'test'; | ||
const stack: number[] = []; | ||
|
||
const result = formatId(id, stack); | ||
|
||
expect(result).toBe('test'); | ||
}); | ||
|
||
it('should handle single stack value', () => { | ||
const id = 'test'; | ||
const stack = [1]; | ||
|
||
const result = formatId(id, stack); | ||
|
||
expect(result).toBe('test-1'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from './format-id'; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
import { TDeepthLevelStack } from '../../types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
export const formatValueDestination = (valueDestination: string, stack: TDeepthLevelStack) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
let _valueDestination = valueDestination; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
stack.forEach((stack, index) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
_valueDestination = _valueDestination.replace(`$${index}`, stack.toString()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
return _valueDestination; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multiple critical improvements needed in the implementation The current implementation has several potential issues that need to be addressed:
Consider this improved implementation: -export const formatValueDestination = (valueDestination: string, stack: TDeepthLevelStack) => {
+export const formatValueDestination = (valueDestination: string | undefined | null, stack: TDeepthLevelStack) => {
+ if (!valueDestination) return '';
+ if (!Array.isArray(stack)) return valueDestination;
+
let _valueDestination = valueDestination;
stack.forEach((stack, index) => {
- _valueDestination = _valueDestination.replace(`$${index}`, stack.toString());
+ const placeholder = `$${index}`;
+ const value = typeof stack === 'object' ? JSON.stringify(stack) : String(stack);
+ while (_valueDestination.includes(placeholder)) {
+ _valueDestination = _valueDestination.replace(placeholder, value);
+ }
});
return _valueDestination;
}; 📝 Committable suggestion
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement the validateOnChange prop functionality
The
validateOnChange
prop is defined but not utilized in the component implementation.Consider implementing validation on value changes when this prop is true: