Emotion: Change innerRef to ref with forwardRef

Created on 9 May 2018  路  14Comments  路  Source: emotion-js/emotion

  • emotion version: 9.1.3
  • react version: 16.3

Relevant code.

const StyledButton = styled.button({
  background: 'blue',
});

class Wrapper extends React.Component {
  constructor() {
    this.buttonRef = React.createRef();
  }

  render() {
    return (
      <StyledButton ref={this.buttonRef} />
    );
  }
}

Problem description:
In my short opinion, most of the time, people try to get a innerRef instead of emotion styled ref. From React 16.3 we can forward inner ref using forwardRef function. It means that we could change the API of emotion more suitable to usecase. If it's not the case, please give me a feedback.

Suggested solution:
Change current ref into emotionRef, emoRef, or someting like those, and also change innerRef to ref using forwardRef.

Help Needed discussion feature request stale

Most helpful comment

I want to do it but we also support Preact and older versions of React(though we're probably going to drop support for them in a future major version) which don't support forwardRef so what I'm thinking we should do is only use forwardRef if it's available.

If you want to submit a PR that would be great!

Also, I don't think we should support getting the ref for the actual styled components since I can't think of a single use for it.

All 14 comments

Would love to see this!

@mitchellhamilton @tkh44
Could you give your opinions?

I want to do it but we also support Preact and older versions of React(though we're probably going to drop support for them in a future major version) which don't support forwardRef so what I'm thinking we should do is only use forwardRef if it's available.

If you want to submit a PR that would be great!

Also, I don't think we should support getting the ref for the actual styled components since I can't think of a single use for it.

Wont it just increase code complexity for a questionable gain? We still need to support innerRef for this major version of emotion & we still need to support it even if forwardRef is available.

Personally I would vote on postponing this change until next major version.

Yeah, I agree with @Andarist here. Just like open source in Ruby on Rails. They support Rails 4, Rails 5, Rails 3, etc. since this is obviously a API Breaking change it should be released properly so people know what they're opting into.

Questionable gain is subjective, it can help simplify code in userland i.e. if you have a parent component that needs a ref of a dynamic child component that can be a React component or styled component.

@Andarist I agree with your last sentence. Not to increase bundle size and not to make a breaking change in a major version, we should defer it to the next major version.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

Actually this issue is resolved in emotion@10.

@Ailrun Where can we found this emotion@10? I only see v9.2.8 in the releases 馃槙

@fabien0102 emotion version 10 uses @emotion scope, so you need to use @emotion/core or similar. You can find full list of packages in next-packages directory of this repo.

@Ailrun Could you clarify how this has been resolved? I'm having an issue finding any relevant changelog in the v10 codebase.

The related code can be found in the current v10.0.0-beta.13 release here:

https://github.com/emotion-js/emotion/blob/9a2585a65d1b170e2e1f4c60ff8f1b4be860f1db/packages/styled-base/src/index.js#L133-L141

@acnebs
You can check real changelog here, but it's WIP.
You can check progress of it in a related PR.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

DarkoKukovec picture DarkoKukovec  路  28Comments

RoystonS picture RoystonS  路  50Comments

Enalmada picture Enalmada  路  27Comments

jamiewinder picture jamiewinder  路  24Comments

kylegach picture kylegach  路  63Comments