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

[datetime2] fix: non-nullable onChange adapter #5740

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Nov 9, 2022

Changes proposed in this pull request:

It's common to see code like this in a function component using DateInput:

const handleChange = React.useCallback((newDate: Date) => {
    // ...
}, [/* some dependencies */]);
return <DateInput2 onChange={handleChange} />;

When this gets migrated to DateInput2 using our automated codemods, it becomes:

const handleChange = React.useCallback((newDate: Date) => {
    // ...
}, [/* some dependencies */]);
return <DateInput2 onChange={DateInput2MigrationUtils.onChangeAdapter(handleChange)} />;

... which nullifies the benefits of useCallback since a new function is being created every time. A better pattern would be:

const handleChange = React.useCallback(
    DateInput2MigrationUtils.onChangeAdapter((newDate: Date) => {
        // ...
    }),
    [/* some dependencies */]
);
return <DateInput2 onChange={handleChange)} />;

... but that did not type check before this PR because of the loose input/output types for the adapter which included undefined.

This change fixes the type of DateInput2MigrationUtils.onChangeAdapter so the last code example works.

@adidahiya adidahiya requested a review from styu November 9, 2022 19:44
@blueprint-bot
Copy link

[datetime2] fix: non-nullable onChange adapter

Previews: documentation | landing | table | demo

@adidahiya adidahiya merged commit 67edc02 into develop Nov 9, 2022
@adidahiya adidahiya deleted the ad/fix-dateinput2-adapter-types branch November 9, 2022 20:06
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.

None yet

2 participants