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

Support for forwardRef components #187

Open
luisherranz opened this issue May 24, 2020 · 5 comments
Open

Support for forwardRef components #187

luisherranz opened this issue May 24, 2020 · 5 comments

Comments

@luisherranz
Copy link

Is your feature request related to a problem? Please describe.

Right now with react-easy-state we can make any functional component or class component reactive, but there's a third type of component called forwardRef that is not supported.

const state = store({
  color: "red",
});

// This is a normal component, reactive thanks to `view`.
const NormalComp = view(() => (
  <h3 style={{ color: state.color }}>Normal component</h3>
));

// This is a forwardRef component.
// We can't make it reactive, `view` doesn't work in this case.
const ForwardComp = React.forwardRef((props, ref) => (
  <h3 ref={ref} style={{ color: state.color }}>
    Forward component
  </h3>
));

// This is a forwardRef component from a library.
// We can' make it reacive either, `view` doesn't work in this case.
const H3 = styled.h3`
  color: ${state.color};
`;

Describe the solution you'd like

Even though this forwardRef components are not very common and mostly used by library authors, I think it could be great to have the ability to make them reactive as well.

Describe alternatives you've considered or seen elsewhere

I think the main alternative is to wrap this components inside an additional normal component that is reactive, extract the props that the forwardRef component needs, pass them down and then when it rerenders, rerenders the forwardRef component children as well.

Additional context

This forwardRef pattern is common on style libraries like styled-components or emotion. For example, this is the implementation of styled in Emotion: https://github.com/emotion-js/emotion/blob/6cb8d15438b01b8623af42f304d5ea3032332187/packages/primitives-core/src/styled.js#L33

The React docs for this are here: https://reactjs.org/docs/forwarding-refs.html

I have made a codesandbox with an example of the issue and the forwardRef components: https://codesandbox.io/s/res-and-forwardref-components-74mvm?file=/src/App.js

@luisherranz
Copy link
Author

It looks like Mobx's observer function (the equivalent of view) supports this but with an additional option, so it's only opt-in: https://mobx-react.js.org/observer-hoc

I don't know if that can be avoided, and make it "just work". When using an external library like Emotion, people is not aware that the components created by the library are not regular components but forwardRef components.

@solkimicreb
Copy link
Member

This is a pretty valid bug, thanks 🙂. I will look into it for the v7 release.

@luisherranz
Copy link
Author

Awesome! Thanks @solkimicreb 😄

@luisherranz
Copy link
Author

luisherranz commented May 24, 2020

I've studied this a little bit. It looks like it's easy to detect when a component is a forwardRef using:

baseComponent["$$typeof"] === Symbol.for("react.forward_ref");

I don't know at which version those symbols where introduced because the Mobx library has a fallback to creating a forwardRef component on the fly to extract it when it's not available.

They claimed they had some problems with react-is, but it's an official Facebook package.

Then, the render is on the render prop:

baseComponent["render"];

It's interesting to read both the observer code of mobx-react and mobx-react-lite:

The observer from mobx-react doesn't have the option to opt-in to forwardRef, it detects if the component is a forwardRef or not by itself. It wraps the forwardRef component with an <Observer> component. Something like this:

if (baseComponent["$$typeof"] === Symbol.for("react.forward_ref"))
  return React.forwardRef((args) => (
    <Observer>{() => baseComponent["render"].apply(undefined, args)}</Observer>
  ));

I don't quite understand why they use <Observer> here to be honest.

On the other hand, the observer from mobx-react-lite needs the opt-in option, and when that is present they do:

const memoComponent = memo(forwardRef(wrappedComponent));

instead of just

const memoComponent = memo(wrappedComponent);

In wrappedComponent it seems like they are always passing ref as a second argument, independent on the forwardRef opt-in option:

const wrappedComponent = (props, ref) => {
  return useObserver(() => baseComponent(props, ref));
};

I don't quite get why they are doing all that instead of just going with a straightforward approach:

  • If baseComponent is a function component, return a function component.
  • If baseComponent is a forwardRef component, return a forwardRef component.

@kutoman
Copy link

kutoman commented Feb 9, 2021

have there been any progress concerning this?

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