Skip to content

Conversation

@ddiestra
Copy link
Contributor

Description

  • Add a new parameter to trigger validation on Set Value

Testing required outside of automated testing?

  • Not Applicable

Screenshots (if appropriate):

  • Not Applicable

Rollback / Rollforward Procedure

  • Roll Forward
  • Roll Back

Reviewer Checklist

  • Description of Change
  • Description of outside testing if applicable.
  • Description of Roll Forward / Backward Procedure
  • Documentation updated for Change

@ddiestra ddiestra requested a review from a team as a code owner January 14, 2026 15:34
@ddiestra ddiestra force-pushed the eng-9638-add-new-validate-parameter branch from 39151e6 to 97cdd3c Compare January 14, 2026 15:36
kevinperaza

This comment was marked as outdated.

kevinperaza

This comment was marked as outdated.

@kevinperaza
Copy link
Contributor

Hey Diego - I took a look at this and I think we're overcomplicating it with the boolean parameter approach. The real issue is architectural: setValue bypasses the validation flow that user input goes through.

The Root Cause

When a user types in an element, it triggers _onChange → validation happens.
When code calls setValue, it bypasses _onChange → no validation.

That's the actual bug. setValue should behave exactly like user input.

Cleaner Solution: Dependency Injection

Instead of adding boolean params and global handler registries, we should just pass _onChange to useBtRef:

// In useBtRef.ts
type UseBtRefProps = {
  btRef?: ForwardedRef<BTDateRef | BTRef>;
  elementRef: RefObject<TextInput>;
  id: string;
  setElementValue: Dispatch<SetStateAction<string>>;
  type?: ElementType;
  onChange?: (value: string) => void;  // NEW: Accept the onChange handler
};

export const useBtRef = ({
  btRef,
  elementRef,
  id,
  type,
  setElementValue,
  onChange,  // NEW
}: UseBtRefProps) => {
  useEffect(() => {
    const valueSetter: ValueSetter = (val) => {
      const formattedValue = valueFormatter(val);
      
      // Update the display
      setElementValue(formattedValue);
      
      // Always trigger validation (just like user input does)
      if (onChange) {
        onChange(formattedValue);
      }
    };

    const newBtRef = createBtRef({
      id,
      elementRef,
      valueSetter,
      type,
    });

    updateRef(btRef\!, newBtRef);
  }, [btRef, elementRef, id, type, setElementValue, onChange]);
};

Then in the element hooks (e.g., CardNumberElement.hook.ts):

const { _onChange, _onBlur, _onFocus } = useUserEventHandlers({
  // ... existing props
});

useBtRef({
  btRef,
  elementRef,
  id,
  setElementValue,
  type,
  onChange: _onChange,  // NEW: Pass the handler directly
});

Why This Is Better

  1. No global state registry - Uses clean dependency injection
  2. No boolean parameters - setValue always does the right thing
  3. Consistent behavior - setValue behaves exactly like user input
  4. Simpler - No conditional logic, no new stores
  5. Type signature doesn't change - setValue stays as setValue(val)

The key insight: setValue isn't doing something "extra" by validating - it should have been going through the normal flow all along. The boolean param was treating validation as opt-in, when it should be the default behavior.

What do you think? Want me to pair on this refactor?

@ddiestra ddiestra force-pushed the eng-9638-add-new-validate-parameter branch from 97cdd3c to 5554d9d Compare January 15, 2026 22:23
@ddiestra ddiestra requested a review from kevinperaza January 15, 2026 22:23
@ddiestra ddiestra force-pushed the eng-9638-add-new-validate-parameter branch from 5554d9d to 0a5cc7b Compare January 15, 2026 22:24
@ddiestra ddiestra merged commit aa5a10a into master Jan 21, 2026
4 checks passed
@ddiestra ddiestra deleted the eng-9638-add-new-validate-parameter branch January 21, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants