Skip to content

Commit

Permalink
Refactor TextareaAutosizeProps
Browse files Browse the repository at this point in the history
  • Loading branch information
Andarist committed Jun 4, 2020
1 parent 017fdf4 commit 61ca826
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .changeset/cuddly-meals-kick.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
'react-textarea-autosize': patch
---

`ref` got excluded from `TextareaAutosizeProps`. It being included has caused problems when using `TextareaAutosizeProps` to declare wrapper components and there seems to be no value in having it included in this type.
`TextareaAutosizeProps` are now based on `React.TextareaHTMLAttributes<HTMLTextAreaElement>` instead of `JSX.IntrinsicElements['textarea']`. The latter one includes a type for `ref` attribute and it being included as part of `TextareaAutosizeProps` has caused problems when using `TextareaAutosizeProps` to declare wrapper components. This is also more semantically correct as `ref` shouldn't be a part of `props`. It's rather accepted by a particular JSX element and in case of the `react-textarea-autosize` this is the type of the exported component which is `React.ForwardRefExoticComponent<TextareaAutosizeProps>` (a result of `React.forwardRef` call).
5 changes: 5 additions & 0 deletions .changeset/hungry-otters-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'react-textarea-autosize': patch
---

`maxHeight` and `minHeight` has been disallowed as part of `TextareaAutosizeProps['style']`. The intention to do that was there since the v8 release but it was not implemented correctly and allowed those to slip into the mentioned type.
9 changes: 4 additions & 5 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@ import getSizingData, { SizingData } from './getSizingData';
import { useComposedRef, useWindowResizeListener } from './hooks';
import { noop } from './utils';

type TextareaProps = React.TextareaHTMLAttributes<HTMLTextAreaElement>;

type Style = Omit<
NonNullable<JSX.IntrinsicElements['textarea']['style']>,
NonNullable<TextareaProps['style']>,
'maxHeight' | 'minHeight'
> & {
height?: number;

This comment has been minimized.

Copy link
@Haaxor1689

Haaxor1689 Jun 5, 2020

Why is the height prop redefined here? I don't see it being used anywhere in code and also shouldn't it be omitted just like min/max height?

This comment has been minimized.

Copy link
@Andarist

Andarist Jun 5, 2020

Author Owner

It's to allow SSRing it (to avoid FOUC), but from the moment of the initial render this component's logic takes over and sets it here:

node.style.height = `${height}px`;

This comment has been minimized.

Copy link
@Haaxor1689

Haaxor1689 Jun 5, 2020

Why change it's type then? It should also accept string values.

This comment has been minimized.

Copy link
@Andarist

Andarist Jun 5, 2020

Author Owner

Well, a number pushes you to actually provide a meaningful value which should be the same one that we are going to calculate after mount. Allowing arbitrary string values (including auto) has a higher chance of a consumer providing incorrect value - or to rephrase: one that might cause FOUC. This is at least the intention behind this, I'm happy to reconsider this.

This comment has been minimized.

Copy link
@kolking

kolking Dec 1, 2023

The redefined height type doesn't match CSSProperties type, so when you try to pass a style object of CSSProperties type (which is correct), you'll get a typescript error because height types are incompatible. Below is how to reproduce the issue:

const style: React.CSSProperties = { color: 'red' };

<TextareaAutosize style={style} />

I agree with @Haaxor1689 that the height type should be left unchanged.

};

export type TextareaAutosizeProps = Omit<
JSX.IntrinsicElements['textarea'],
'ref'
> & {
export type TextareaAutosizeProps = Omit<TextareaProps, 'style'> & {
maxRows?: number;
minRows?: number;
onHeightChange?: (height: number) => void;
Expand Down

0 comments on commit 61ca826

Please sign in to comment.