Repository navigation
feat(useInputState): add previous state to useInputState transform function #574
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
base: main
Are you sure you want to change the base?
Changes from all commits
8497d9d
b26fd6e
77a8fad
724068c
560e3ee
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,5 @@ | ||
| --- | ||
| 'react-simplikit': minor | ||
| --- | ||
|
|
||
| Pass the previous state value as the second argument to useInputState's transformValue callback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,9 @@ type InitialValue = string | (() => string); | |
| * The returned `onChange` handler works with both `<input>` and `<textarea>` elements. | ||
| * | ||
| * @param {string | (() => string)} [initialValue=""] - The initial value of the input. Defaults to an empty string (`""`). | ||
| * @param {(value: string) => string} [transformValue=(v: string) => v] - A function to transform the input value. | ||
| * Defaults to an identity function that returns the input unchanged. | ||
| * @param {(nextValue: string, previousValue: string) => string} [transformValue=(nextValue: string) => nextValue] | ||
| * - A function to transform the input value. The previous state value is available as its second argument. | ||
| * Defaults to an identity function that returns the next input unchanged. | ||
| * | ||
| * @returns {[value: string, onChange: ChangeEventHandler<HTMLInputElement | HTMLTextAreaElement>]} A tuple containing: | ||
| * - value `string` - The current state value; | ||
|
|
@@ -28,19 +29,22 @@ type InitialValue = string | (() => string); | |
| * ); | ||
| * } | ||
| */ | ||
| export function useInputState(initialValue: InitialValue = '', transformValue: (value: string) => string = echo) { | ||
| export function useInputState( | ||
| initialValue: InitialValue = '', | ||
| transformValue: (nextValue: string, previousValue: string) => string = echo | ||
| ) { | ||
| const [value, setValue] = useState(initialValue); | ||
|
|
||
| const handleValueChange: ChangeEventHandler<InputLikeElement> = useCallback( | ||
| ({ target: { value } }: ChangeEvent<InputLikeElement>) => { | ||
| setValue(transformValue(value)); | ||
| ({ target: { value: nextValue } }: ChangeEvent<InputLikeElement>) => { | ||
| setValue(transformValue(nextValue, value)); | ||
| }, | ||
| [transformValue] | ||
| [transformValue, value] | ||
| ); | ||
|
Comment on lines
38
to
43
Member
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. This works well I think. but one thing that im not sure about it 'onChange gets a new reference every time the value change' (since value is now in deps) we could keep it stable with 'usePreservedCallback' what do you think about it?
Contributor
Author
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. As you pointed out, the current implementation creates a new I tried using Here’s the relevant part of the reproduction. const [value, setValue] = useState('abcd');
const triggered = useRef(false);
const onChange = usePreservedCallback((nextValue: string) => {
setValue(transformValue(nextValue, value));
});
useLayoutEffect(() => {
if (value === 'abcde' && !triggered.current) {
triggered.current = true;
onChange('abcdef');
}
}, [value, onChange]);After typing (This reproduction deliberately calls the handler during a layout Effect. I didn’t observe the issue during normal typing, so I can’t say how likely it is in typical usage.) Given that result, I think it’s better to accept a new |
||
|
|
||
| return [value, handleValueChange] as const; | ||
| } | ||
|
|
||
| function echo(v: string) { | ||
| return v; | ||
| function echo(nextValue: string) { | ||
| return nextValue; | ||
| } | ||
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.
The default function’s parameter was renamed in the documentation, but the implementation still uses the old name.
Before
[transformValue=(v: string) => v]After
[transformValue=(nextValue: string) => nextValue]Would it make sense to update the implementation for consistency? I think the
echofunction should be updated as well.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.
Thanks for pointing this out! 👍
I agree that the implementation should be consistent with the documentation.
I update the parameter name in the default function and the
echofunction as well.