static methods not hoisted by injectIntl

[shriah]

When I tried using injectIntl I was not able to access the static method of the child component. The connect component of react-redux is using hoist-non-react-statics for the same purpose. May be doing the same with injectIntl will be helpful

[ericf]

For the time being you can use hoist-non-react-statics in your app code to get around the issue. I'm curious to see the actual real world example where you have a module importing a component and calling its static memebers. Could you show me so I have a better idea?

[shriah]

I am using a static method along with redux-router to make api calls and wait for them before rendering the components in server side Example

This is how I am using it in server side Example

[ericf]

@shriah I spent some time digging into this, and I'm not sure why the fetchData() pattern is using a react component's statics for this. Also, it looks like react-router is moving away from using statics.

I'm thinking this simply a limitation of CommonJS, since you have to hang everything off the "default" export in CommonJS, people want the main export to be the component and therefore have to hang things off the constructor. With ES6 modules you can export the component as the default export and a function like fetchData() as a named export.

With ES6 modules one would:

export default injectIntl(Component);
export function fetchData() {}

[shriah]

@ericf fetchData() is a user defined function and not related to react-router so this will not be a problem.

The problem with exporting fetchData is that, I have to import all the fetchData from all my components in my server side code to initiate the api calls. With statics I don't have to worry about importing it and it is already there for my use. For each request fetchData call will be made on different components depending on the requested page.

I also feel using HOC should not restrict the functionality of the wrapped component.

I am not sure there is a better way to do this without static functions

Promise.all(

      // Get array of components that will be rendered from the router state.(Not all components)
      // The list of components will be different for each request
      initialState.router.components
          // We need only components with `fetchData` method
          .filter(component => component.fetchData)
          // Fetching the data!
          // `args` is object with required arguments for `fetchData` method
          .map(component => component.fetchData({ /* args */ }))
    );

[itsmepetrov]

I agree with @shriah. I have the same problem with injectIntl in server side.

[ericf]

The problem with exporting fetchData is that, I have to import all the fetchData from all my components in my server side code to initiate the api calls. With statics I don't have to worry about importing it and it is already there for my use.

How is initialState.router.components populated? Where does the array of components come from? How does the fetchData functions relate to the component? It seems like this is related to the flux system and not the actual component itself.

Another related reference point is Facebook's Relay project, which has a HOC factory function similar to injectIntl() which also doesn't alias the statics.

[shriah]

I just want to stress that this matters to me mostly because of server side rendering.

initialState is from redux store which has the state of the application.

initialState.router is the added by redux-router and it has the redux-router related information. For example initialState.router.components has the components that will be rendered for the requested page.

fetchData method will dispatch actions that will populate the state will data needed to render the component, before the component is rendered. This method is used only in server side. Normally you will do the same thing in componentWillMount in client side. fetchData is just one of the pattern used in server side to ensure that you send a fully rendered component to a request.

Reference for other projects that do hoist the statics. fluxible-addons-react and fluxible-router are there.

[ericf]

@shriah I will continue to investigate this as time allows because I wanted to understand the root of the issue and code patterns. For now you can apply call injectIntl before adding your fetchData static method. Thanks for providing the additional info so I know where to look…

[akx]

Thanks @ericf for pointing me in the direction of this discussion from my PR #219.

My use case is pretty much exactly the same as @shriah 's; you can actually see the implementation here. The manual hoisting is done e.g. here, here and here...

Using hoist-non-react-statics would be a more flexible way to go about doing what my PR does, but I felt like going with a less invasive implementation and mirror what react-redux's connect() HOC already does.

(Sidebar: Ultimately, it feels like it'd be better for the community to standardize on a better protocol for HOCs that wouldn't involve wrapping wrappers in wrappers; right now the kerrokantasi-ui project linked above already has two layers of HOC wrapping for each component, namely react-redux and react-intl, and it's already complicating testing.)

[ericf]

I'm closing this as 'wontfix' for now.

[Strate]

Static methods should not be touched by any decoration, this is very strange, guys.

[Strate]

I write a component:

export default class ReactComponent extends React.Component {
  static Field = 123
}

Imagine I have a lot of usages for ReactComponent.Field in my code. And, once upon a time, I needed to intl api in my component. Ok, I use a es7 decoration:

import {injectIntl} from "react-intl"

@injectIntl
export default class ReactComponent extends React.Component {
    static Field = 123
}

And.. My code stops to work. Why? Because @injectIntl hides all static properties.
What should I do? Expected - nothing, my code should works.
Now I should: rewrite all my code, or I should write a custom function decorator.

From my point of view, any decorator should extend class functionality. Not decrease.

[TheXardas]

I have the same issue!
Some imported component should not make changes to component in which it's been injected. That kind of behaviour is totally unexpected.
That's an obvious violation of Inversion of Control principle.
That just smells!

[ericf]

injectIntl() isn't a decorator, it doesn't return the class/component passed in. Instead, it returns a new component that renders the component that was passed in.

---

I don't consider it a bug, but a design choice. Forwarding statics can be brittle and isn't required for React's component rendering protocol. I did research this and found that many of the usages where people want statics forwarding were for Flux related metadata and functions are not part of the component's own encapsulation; i.e. the component itself never accesses these statics, nor does React the framework. It seems that statics is used as a convenience in a component created with React.createClass() that's defined as a CommonJS module so that people don't have to type FooComponent. multiple times when defining their statics.

Defining a ES6 class or function for a React component is declarative and statically analyzable. When you start forwarding a class'/component's static members, it becomes a dynamic, runtime operation — similar to a mixin. This feature was removed from the public version of Relay for this reason: facebook/relay#154

I'd prefer the code be explicit, and written directly in the source code rather than having a dynamic operation mutate the class at runtime to create something different than what is visually defined in the source code.

I think the Flux related metadata could actually be moved to explicit exports of the module. When using ES6 modules, it's arguably cleaner and and more explicit to create a Controller View with these exports as it separates the component from the Flux system:

export default class Foo extends React.Component {}
export const componentConfig = {};
export function initializeAction() {}

I like this pattern, especially for the initializeAction(), fetchData(), etc. since it really has nothing to do with the React Component itself (in that the component would never call this method on itself), rather it's something higher-level than the component since it's about fetching data that will eventually be passed in to the component instance as props. It's a part of the module, not the React Component.

[TheXardas]

injectIntl() isn't a decorator, it doesn't return the class/component passed in. Instead, it returns a new component that renders the component that was passed in.

But it's an internationalization component. It's not "cut all static members" module.

[Strate]

Let abstract from React Components. Static methods is not about metadata or similar things.
static properties usually used for constants, for enums (in typescript), or similar things.

injectIntl() isn't a decorator, it doesn't return the class/component passed in. Instead, it returns a new component that renders the component that was passed in.

Adding @injectIntl I want to say "Add intl api to this component", not "create new component with intl api"

[ericf]

But it's an internationalization component. It's not "cut all static members" module.

@TheXardas I don't understand your argument, or what you're constructively adding here.

Adding @injectIntl I want to say "Add intl api to this component", not "create new component with intl api"

@Strate again, not a decorator. It's literally a function that returns a component. What's a concrete example of defining component statics that don't make more sense to be explicit exports instead? Remember, propTypes and defaultProps are still respected since the wrapper will render the wrapped component.

[Strate]

@ericf ok, I think I understood you.
I'll try to rethink about static properties later, but now I'll use my own @injectIntl function, which does exactly what it means: injects intl to props via modifying contextTypes and propTypes

[quicksnap]

FWIW Here's a code example of an alternative as a 'workaround', as mentioned above.

@injectIntl
@connect( /*...*/ )
export default MyComponent extends Component {
  render() { /*...*/ }
}

MyComponent.fetchData = () => { /*...*/ }

While I had the same issue, and it would be pleasant for it to magically go away, @ericf's reasoning is sound--runtime mutation of the class is seems more brittle. Since injectIntl and connect have similar use cases in connecting context to components, I'm curious on the divergence in decisions.

[mariuscoto]

@quicksnap can you please also provide an ES5 version of the 'workaround' ?

[kellyrmilligan]

const myinjectedComponent = injectIntl(class Mycomponent extends Component {
})`

myInjectedComponent.fetchData = 

[ericketts]

for you flow fans:

import * as React from 'react';
import {injectIntl as baseInjectIntl, type IntlShape} from 'react-intl';
import hoistNonReactStatic from 'hoist-non-react-statics';

type TOpts = {intlPropName: string, withRef: boolean};

export default function injectIntl<Props, SanitizedProps: $Diff<Props, {intl: IntlShape}>>(
  BaseComponent: React.ComponentType<Props>,
  options?: TOpts
): React.ComponentType<SanitizedProps> {
  const WrappedComponent = baseInjectIntl(BaseComponent, options);
  hoistNonReactStatic(WrappedComponent, BaseComponent);
  return WrappedComponent;
}

export type {IntlShape} from 'react-intl';

I re-export the IntlShape so you need only import a single file for both props definition and the call to wrap the component.