Skip to content
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

Extend TypeScript interfaces for all migrated components to include intrinsic HTML props #583

Open
7 tasks
Tracked by #168
edda opened this issue Nov 3, 2024 · 0 comments
Open
7 tasks
Tracked by #168
Assignees

Comments

@edda
Copy link
Contributor

edda commented Nov 3, 2024

Description

While reviewing the Select component's PR, it was noted that the FormHint component required an update to handle intrinsic HTML props correctly. Specifically, the component needed an id prop for its use within the Select component:

<FormHint text={helptext} id={helptextId} />

This change exposed a type error since the FormHint interface did not explicitly define id. However, id is an intrinsic HTML attribute for the

element that FormHint renders.

To avoid cluttering component interfaces with numerous intrinsic HTML attributes, our convention is for all components to have a generic props property obeject which is used to catch all of the internal HTML element's props and pass them on to it.

The proposed way to deal with this in our typescriptified components is to extend the interface with React.ComponentPropsWithoutRef<'element'> or React.ComponentPropsWithRef<'element'>, ensuring all intrinsic HTML props are passed correctly.

For example, the FormHint component was updated as follows:

export interface FormHintProps extends React.ComponentPropsWithoutRef<"div"> {
  /** additional props here **/
}

This approach ensures all valid HTML props are handled without explicitly adding each to the component's interface.

Task

  • Review all components that have already been migrated to TypeScript.
  • Extend each component’s interface with React.ComponentPropsWithoutRef<'element'> or React.ComponentPropsWithRef<'element'>, as appropriate, to capture intrinsic HTML props. As 'element' choose the element which receives the {...props}
  • In cases where a component doesn't apply {...props} to an HTML element but to another component decide on a case-by-case basis what makes sense
  • Ensure this change does not introduce breaking changes in existing component usage.

Acceptance Criteria

  • Each migrated component should accept all intrinsic HTML props applicable to its rendered HTML element (or component).
  • Component interfaces should remain clean, only listing additional props specific to that component.
  • No breaking changes are introduced.

References

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

No branches or pull requests

2 participants