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

withI18n does not copy over static methods #166

Closed
elios264 opened this issue Feb 21, 2018 · 5 comments
Closed

withI18n does not copy over static methods #166

elios264 opened this issue Feb 21, 2018 · 5 comments

Comments

@elios264
Copy link

As stated in React docs when creating a HOC, you need to copy over the static methods,

Some of my components declare static properties, but they are undefined when using withI18n

This fix is simple, the only addition is to call hoistNonReactStatic over the hoisted and original component

@tricoder42
Copy link
Contributor

Do you think this should be baked to jsLingui or simply adding a note to documentation is enough?

import hoistNonReactStatic from 'hoist-non-react-statics';

class Page extends React.Component { ... }

export default hoistNonReactStatic(withI18n()(Page), Page)

I'm not sure if we should do it implicitly. Larger bundle is just one disadvantage, even though the package seems to be rather small.

@vonovak
Copy link
Collaborator

vonovak commented Feb 21, 2018

I think it is good practice to hoist the statics. I think you may also use render prop instead of a HOC and they don't need hoisting afaik.

@elios264
Copy link
Author

I would add it to jsLingui, because "strictly" speaking any HOC should add additional functionality without removing other and withI18n is removing that feature, react-redux which also provides the connect HOC is also hoisting the static methods and props.

But in any case this can easly be done outside like you stated:
export default hoistNonReactStatic(withI18n()(Page), Page)

And since it is your package the decision is up to you.

@tricoder42
Copy link
Contributor

Make sense, I'll add it to keep HOC behavior consistent.

@tricoder42
Copy link
Contributor

Fixed in latest release v2.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants