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

Remove arity check from ref prop type #57

Merged
merged 1 commit into from Apr 9, 2019

Conversation

Projects
None yet
2 participants
@ahuth
Copy link
Contributor

ahuth commented Apr 8, 2019

In #54 we added a ref prop type, and in #55 added an arity check for callback refs. Unfortunately, that breaks a common pattern of having an empty function as a default value. For example:

function SomeButton({ buttonRef }) {
  return <button buttonRef={buttonRef} />;
}

SomeButton.defaultProps = {
  buttonRef() {}
};

We could leave the arity check and change default values to undefined, instead. However, that's an unknown amount of work, and I think we're better off removing the arity check and accepting functions with any number of arguments.

@ljharb , what do you think?

[patch] `ref`: Remove arity check
Checking for an arity of 1 for callback refs interferes with default values of optional ref props, which oftentimes are empty functions without any args.

@ahuth ahuth referenced this pull request Apr 8, 2019

Merged

Add type parameter for airbnb-prop-types ref objects #34569

8 of 9 tasks complete
@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Apr 9, 2019

Fair point; we could also accept both 0 and 1 but then are we really rejecting anything useful?

@ljharb

ljharb approved these changes Apr 9, 2019

@ljharb ljharb merged commit 8b479ea into airbnb:master Apr 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ahuth

This comment has been minimized.

Copy link
Contributor Author

ahuth commented Apr 9, 2019

Thanks!

@ahuth ahuth deleted the ahuth:refs-3 branch Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.