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

hoistStatics implementaion in withCookies unexpectedly changed after tag 2.2.0 #225

Closed
johannesleander opened this issue Jun 5, 2019 · 2 comments

Comments

@johannesleander
Copy link

johannesleander commented Jun 5, 2019

Attempting to upgrade react-cookie from 2.1.7 to 2.2.0 or above caused our application using the withCookies HOC to break.

After having spent a couple of days of debugging we've come to the conclusion that the React components HOC:ed with withCookies and other third-party HOCs dropped the reference to the innermost component in the property WrappedComponent after having been hoisted in withCookies. As our application relies on WrappedComponent to contain a reference to the base component through the entire hoisting process, it caused major issues for us.

The 2.2.0 release of react-cookies contains this diff with 2.1.7 in withCookies.js:

-  18 Wrapper.displayName = `withCookies(${Component.displayName || Component.name})`;
-  19 Wrapper.WrappedComponent = Component;
-  30   return hoistStatics(Wrapper, Component);
+   47  return hoistStatics(Wrapper, WrapperComponent, { WrappedComponent: true });

For our use-case, this was a breaking change.

Example component to reproduce behavior:

import React from 'react'
import { withCookies } from 'react-cookie';
import { withRouter } from 'react-router-dom';

function MyComponent(props) {
  return <div></div>
}

console.log(typeof MyComponent.WrappedComponent); // 'undefined' (correct)

const ComponentWithCookies = withCookies(MyComponent);

console.log(typeof ComponentWithCookies.WrappedComponent); // 'undefined' (expected 'object')

const ComponentWithRouter = withRouter(ComponentWithCookies);

console.log(typeof ComponentWithRouter.WrappedComponent); // 'object' (correct)

export default ComponentWithRouter;

Question: From what we can tell, the third parameter to hoistStatics is a blacklist of statics not to hoist. Was this change intentional or an oversight? If it was not intentional I would be happy to submit a PR to fix this issue.

Thank you for maintaining this great library!

@eXon
Copy link
Collaborator

eXon commented Jul 3, 2019

It has never been part of the public API, but it is definitively best practices to provide it. I'll put it back and add it to the official doc.

@eXon
Copy link
Collaborator

eXon commented Jul 4, 2019

Fixed in v4.0.1 and added to the documentation!

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

2 participants