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

Accept Ref Objects as Refs? #10581

Closed
sebmarkbage opened this issue Aug 31, 2017 · 6 comments
Closed

Accept Ref Objects as Refs? #10581

sebmarkbage opened this issue Aug 31, 2017 · 6 comments

Comments

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Aug 31, 2017

Reason React uses first class OCaml refs to store mutable values (instead of on this instances).

These are basically just an object with a mutable contents property. These can be updated with callback refs n => ref.contents = n but it would be a nice convenience feature to just have that built-in.

We could also make these first class objects on isomorphic React.

React.createRef = () => ({ contents: null });
class Foo extends React.Component {
  state = {
    myDiv: React.createRef()
  };
  componentDidMount() {
    if (myDiv.contents) {
      myDiv.contents.focus();
    }
  }
  render() {
    return <div ref={this.state.myDiv} />;
  }
}

Basically the implementation would just be:

if (typeof ref === 'function') {
  ref(newValue);
} else if (typeof ref === 'object') {
  ref.contents = newValue;
} else if (typeof ref === 'string') {
  owner.refs[ref] = newValue;
}

This is something that needs to be implemented in the core runtime and not as part of any particular component API since refs cross that boundary.

cc @adamjernst

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Aug 31, 2017

The thing that is special about OCaml is that there is already a first-class language feature for these. So you would by-pass React.createRef() and just use { myRef: ref Node }. There is also syntax sugar for accessing this value. (!state.myRef).focus

@syranide
Copy link
Contributor

Each ref object being allocated separately is not great, but refs are perhaps not prevalent enough for this to really be a concern, and in practice this would probably even end up being faster as most people use the inline fat-arrow syntax which is (currently) inefficient.

Could one imagine a form that would work for arrays of elements too? (i.e. when you want to keep a ref to each of the items in a list) Otherwise, that seems problematic for users to achieve in React if they prefer this style.

@syranide
Copy link
Contributor

I'm guessing it wouldn't be unthinkable to have native support for refs either? It would just act exactly as the value it is referencing, except there's some way of updating the ref, e.g. setRef(myRef, newValue).

@sebmarkbage
Copy link
Collaborator Author

I suppose one issue is that this doesn't actually compile to{contents:...} with BuckleScript. It compiles to an opaque object like [...]. So we wouldn't know the data structure's signature anyway.

So maybe this does indeed have to use ref callbacks as the interop layer.

cc @chenglou

@chenglou
Copy link
Contributor

The record compiles to an array of a single item. It's an internal representation (though unlikely to change). A bit torn...

@gaearon
Copy link
Collaborator

gaearon commented Aug 15, 2018

Out in 16.3

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

4 participants