Skip to content

Conversation

@ompharate
Copy link
Contributor

Closes #6187

Fixed TypeScript type definitions for TextInput component to make all styling and layout props optional as documented in the official Primer documentation. Previously, props like block, contrast, disabled, monospace, sx, width, maxWidth, minWidth, variant, size, and validationStatus were incorrectly treated as required, causing TypeScript errors when using TextInput with minimal props.

Before

❌ Error: missing required props

After

✅ All styling props are optional
✅ Optional props work when provided

Changelog

TextInput: Fixed TypeScript type definitions to make all styling and layout props optional as documented in the official Primer documentation

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Rationale: This is a patch release as it fixes incorrect TypeScript definitions without changing any runtime behavior or introducing breaking changes. The change makes the component easier to use by aligning TypeScript types with the documented API, but doesn't affect existing functionality.

Testing & Reviewing

To test this change:

  1. TypeScript Compilation:

    • Verify that <TextInput /> with no props compiles without TypeScript errors
    • Test that optional props like <TextInput block contrast size="large" /> still work correctly
    • Ensure IntelliSense/autocomplete suggestions show props as optional
  2. Runtime Behavior:

    • All existing functionality should work exactly as before
    • No visual or behavioral changes expected
    • Component should render correctly with and without optional props
  3. Storybook Testing:

    • Check that all existing TextInput stories continue to work
    • Verify no console errors or warnings
    • Test interactive examples to ensure props work as expected
  4. Type Safety:

    • Confirm that providing invalid prop values still triggers TypeScript errors
    • Verify that the Partial<Pick<StyledWrapperProps, ...>> pattern works correctly

Key areas to review:

Merge checklist

Note: Tests, documentation, and Storybook previews are not required for this change as it only fixes TypeScript type definitions without affecting runtime behavior. The existing tests and documentation remain valid. SSR compatibility is confirmed as this change only affects TypeScript types, not component rendering logic.

Copilot AI review requested due to automatic review settings July 19, 2025 19:03
@ompharate ompharate requested a review from a team as a code owner July 19, 2025 19:03
@ompharate ompharate requested a review from joshblack July 19, 2025 19:03
@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2025

🦋 Changeset detected

Latest commit: 4f5f14b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes TypeScript type definitions for the TextInput component by making styling and layout props optional. The change addresses issue #6187 where props like block, contrast, disabled, monospace, sx, width, maxWidth, minWidth, variant, size, and validationStatus were incorrectly treated as required, causing TypeScript compilation errors when using TextInput with minimal props.

Key changes:

  • Wrapped the Pick<StyledWrapperProps, ...> type with Partial<> to make all styling props optional
  • Maintains backward compatibility while fixing TypeScript errors for minimal usage patterns

Updated TypeScript types for TextInput component.
@ompharate
Copy link
Contributor Author

ompharate commented Jul 25, 2025

@joshblack is this ready to merge?

@joshblack
Copy link
Member

Hey @ompharate! I believe so, will get it merged in today 🥳 Thanks again for taking the time to contribute!

@joshblack joshblack added this pull request to the merge queue Jul 28, 2025
Merged via the queue into primer:main with commit 744102b Jul 28, 2025
39 of 40 checks passed
@primer primer bot mentioned this pull request Jul 28, 2025
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.

TextInput extra mandatory props

2 participants