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

Bad prop type declaration and documentation for PlaidLink component's component prop #198

Closed
prometheas opened this issue Oct 1, 2020 · 1 comment
Assignees

Comments

@prometheas
Copy link
Contributor

The problem

The component's documentation and prop types declarations violently disagree with its implementation. When I use the component as documented in the README, I get the following warning in my console:

Warning: Failed prop type: Invalid prop `component` of type `object` supplied to `PlaidLink`, expected `function`.

Given the following implementation in PlaidLink.js:

export const PlaidLink = ({
  component,
  componentProps,
  children,
  ...linkProps
}) => {
  // 🚨 NOTE: the following line doesn't take care to defend against `undefined`
  const Component = component;
  return (
    <Component
      {...componentProps}
      onPress={() => handlePress(linkProps, componentProps)}
    >
      {children}
    </Component>
  );
};

And the following prop type definition for the component prop:

PlaidLink.propTypes = {
  // skip irrelevant bits...

  // Underlying component to render 🚨 NOTE: this fails to support class components
  component: PropTypes.func,

  // everything else...
};

… it is clear that the PlaidLink component requires the component prop, though there is presently no documentation as to the requirements for that component (which I'm assuming exist, as the placement of that component includes an onPress prop, which means using a View is off the table, as it doesn't support that prop.

Further, the prop type declaration only accepts a function, which results in a prop type warning (see my note in the comment addendum to the prop type declaration code sample above) when supplying something like component={TouchableOpacity}, which actually works.

This problem has been around since 3 Dec 2019, introduced by this commit. Notably this change wasn't accompanied by a corresponding update to the components usage documentation the README file (nor has any subsequent update introduced this needed update).

Environment

Plaid Link React Native 5.0.2
ReactNative Version 0.63.2
Occurs on Android ?
Android OS Version n/a
Android Devices/Emulators n/a
Occurs on iOS yes
iOS Version 14
iOS Devices/Emulators all

Steps to Reproduce

Nothing special needed; simply use the component in dev mode and you'll see the problem.

Expected Result

I expected the component prop to be optional, and the PlaidLink component to work properly without it.

Screenshots

No real value to a screen shot of warning text.

Logs

Any relevant logs from Android Logcat or the iOS console including stacktraces.

Code To Reproduce Issue

Again, all you need to do is use the code from the usage documentation to get this result.

@prometheas prometheas changed the title Undocumented break in BC for PlaidLink component by addition of component prop in December 2019 Bad prop type declaration and documentation for PlaidLink component's component prop Oct 1, 2020
@prometheas
Copy link
Contributor Author

UPDATE: I modified this issue's report after noticing that the component does define TouchableOpacity as the default component type to use, in the absence of an explicit value 😅

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

3 participants