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

Add React.createRef() as the migration path for string refs #11973

Closed
gaearon opened this issue Jan 5, 2018 · 9 comments
Closed

Add React.createRef() as the migration path for string refs #11973

gaearon opened this issue Jan 5, 2018 · 9 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Jan 5, 2018

Creating this issue to track #11555. I intend to close the PR as it's outdated, but we probably want to turn it into a real RFC and potentially get it in during 16.x.

@taion
Copy link

taion commented Jan 7, 2018

As an alternative to adding new React API, could this just be moved into userspace?

In the OP from #11555, we could amend the example as:

import createRef from 'create-react-ref';

class MyComponent extends React.Component {
  render() {
    return <div><input type="text" ref={createRef(this, 'div')} /></div>;
  }

  componentDidMount() {
    this.refs.div.value.focus();
  }
}

As a really dumb implementation:

export default function createRef(inst, refName) {
  if (!inst.refs) {
    inst.refs = {};
  }

  if (!inst.stringRefFuncs) {
    inst.stringRefFuncs = {};
  }

  if (!inst.stringRefFuncs[refName]) {
    inst.stringRefFuncs[refName] = c => {
      inst.refs[refName] = c;
    };
  }

  return inst.stringRefFuncs[refName];
}

This may not literally work as-is given that I think touching this.refs is weird, but it still seems like this API could live outside of React core.

It would even be possible to write a Babel transform to do this automatically.

@taion
Copy link

taion commented Jan 7, 2018

Alternatively:

function createRef() {
  function ref(c) {
    ref.value = c;
  }
  
  return ref;
}

I don't know if the edge cases exactly match #11555, but this can live in userspace, and doesn't require any changes to the reconciler.

Unless per #11555 (comment) and #11555 (comment), the intention is for users of "vanilla" function refs to migrate to this scheme as well.

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 7, 2018

What is the benefit of moving it out? File size? (Presumably it would be small enough and the vast majority of libs would need it anyway.)

@taion
Copy link

taion commented Jan 7, 2018

I'm not sure. I didn't quite follow the conclusion from #11555 (comment).

Is the idea that, going forward, most ref users will use createRef rather than attaching a callback ref? Or is this a convenience thing for people currently using string refs, but that going forward people should still mostly use callback refs?

If it's a convenience thing, it seems like putting this in user space reduces the React API surface area.

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 7, 2018

I think the idea is that most people will use createRef, and callback refs are relegated to being a power user feature since they’re more cumbersome and harder to explain.

@taion
Copy link

taion commented Jan 7, 2018

That makes sense. Thanks!

@strayiker
Copy link

What is the benefit of moving it out?

Maybe it will be less attractive for use, because it's another dependency? In this case, it's not so obvious what will be preferable for use. That makes sense?

@TrySound
Copy link
Contributor

TrySound commented Feb 6, 2018

@strayiker use rfc for comments

@trueadm trueadm closed this as completed Feb 6, 2018
@trueadm trueadm reopened this Feb 6, 2018
@gaearon
Copy link
Collaborator Author

gaearon commented Aug 9, 2018

Shipped in 16.3

@gaearon gaearon closed this as completed Aug 9, 2018
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

5 participants