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

Unnecessary code when calling event handler props #5044

Open
laug opened this issue Aug 22, 2024 · 0 comments
Open

Unnecessary code when calling event handler props #5044

laug opened this issue Aug 22, 2024 · 0 comments

Comments

@laug
Copy link
Contributor

laug commented Aug 22, 2024

Throughout index.tsx there are many instances of code similar to the below:

onChange
    ? onChange(changedDate, event)
    : DatePicker.defaultProps.onChange;

This is problematic for several reasons:

  • The 'else' line of the ternary does nothing. DatePicker.defaultProps.onChange references a function, but the function is not called as the brackets () are missing.
  • Even if we did add the brackets, DatePicker.defaultProps.onChange is an empty function, so calling it achieves nothing.
  • The code is therefore equivalent to just onChange && onChange(changedDate, event) which can be more simply written onChange?.(changedDate, event)
  • Providing an empty function, as opposed to undefined, as the default prop, does not appear useful, as the calling code always checks that the prop is truthy (not undefined) before calling it. We are forced to check the function is not undefined (rather than relying on the default prop always being defined) as the user can pass null as the event handler prop, and we do not want to fail in that case.
  • With the above code change, we no longer reference the default prop. Also, calling an empty function will never have a different effect than doing nothing at all.

So it would seem reasonable to refactor these 3 lines of code into one in the many places where they appear, and to remove the default prop empty functions as they are not needed.

This refactoring will then make it easier to implement further changes, such has correctly handling the input of date ranges in the textbox, as requested in issues #3596, #3906, #4002.

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

1 participant