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 Ref with react ref api #126

Open
xialvjun opened this issue Jun 19, 2018 · 10 comments
Open

add Ref with react ref api #126

xialvjun opened this issue Jun 19, 2018 · 10 comments

Comments

@xialvjun
Copy link

xialvjun commented Jun 19, 2018

<Ref>
  {ref => (
    <React.Fragment>
      <button onClick={e => ref.current.openModal()}>open</button>
      <Modal ref={ref}>
        <div>model_content</div>
        <button onClick={e => ref.current.closeModal()}>close</button>
      </Modal>
    </React.Fragment>
  )}
</Ref>
@egoarka
Copy link
Contributor

egoarka commented Jul 19, 2018

Check this out: https://codesandbox.io/s/4z26pmkp87
not sure that it is safe to use

@xialvjun
Copy link
Author

xialvjun commented Jul 20, 2018

No, you should not use SFC. Using SFC, it's safe, but it's slow.
Use Component instead.

export class Ref extends Component {
  ref = React.createRef();
  render() {
    return this.props.children(this.ref);
  }
}

And besides, another feature request:

export class Instance extends Component {
  constructor(props) {
    super(props);
    props.init && props.init(this);
  }
  render() {
    return this.props.children(this);
  }
}

const list = [xxxx];
const to_render = <Instance init={ins => ins.audios = {}}>
  {ins => <ul>
    {list.map(item => <li key={item.id}>
      <audio preload="none" src={item.url} ref={ele => ins.audios[item.id] = ele} />
      <button onClick={e => ins.audios[item.id].play()}>play</button>
    </li>)}
  </ul>}
</Instance>

Instance is not only a Multi Ref Component, in fact, with it, we can do everything, like cache previous state, define lifecycles...

@TrySound
Copy link
Collaborator

I'd say you should go with classes :) Component instance is bad abstraction because implementation detail is part of the api.

@xialvjun
Copy link
Author

@TrySound you are talking about Instance component?
But to write a Component Class, I need to go out of JSX, write the class, then use it in JSX.
With Instance component, I can just write it in JSX.

Why I think just write in JSX is important?

Because write a class is much harder than write a vnode: well, write a class is not hard, but compared to write a vnode, you'll feel it much harder.

@TrySound
Copy link
Collaborator

I know it's hard, but mutable instance with methods as first class citizen is not the best api for immutable tree. Instance has a lot of stuff which is implementation detail in context of render prop. I would prefer to mutate state with values you need.

@xialvjun
Copy link
Author

But in some cases, we do really need mutable instance which won't cause a update when changing its state.

Well, without Instance, how to make a Multi Ref Component

@TrySound
Copy link
Collaborator

Show me the case please

@xialvjun
Copy link
Author

Code in #126 (comment) showed the case.
Copy it here:

const list = [xxxx];
const to_render = <Instance init={ins => ins.audios = {}}>
  {ins => <ul>
    {list.map(item => <li key={item.id}>
      <audio preload="none" src={item.url} ref={ele => ins.audios[item.id] = ele} />
      <button onClick={e => ins.audios[item.id].play()}>play</button>
    </li>)}
  </ul>}
</Instance>

@TrySound
Copy link
Collaborator

It's okay to mutate state.

const list = [xxxx];
const to_render = (
  <State initial={{ audios: {} }}>
    {({ state }) => (
      <ul>
        {list.map(item => (
          <li key={item.id}>
            <audio preload="none" src={item.url} ref={ele => state.audios[item.id] = ele} />
            <button onClick={e => state.audios[item.id].play()}>play</button>
          </li>
        )}
      </ul>}
  </State>
)

Or if you don't want to mutate state you may create mutable object on instance. Just don't pass instance with all its methods. Operate with data only here, mutable and immutable.

class Mutable extends React.Component {
  mutable = this.props.initial;

  render() {
    return this.props.children(this.mutable)
  }
}

@xialvjun
Copy link
Author

Well, it's a debate between two API offering philosophy: I offer these apis because I can and I offer these apis because you just need them.

In fact, I don't know which philosophy is better... -_-!!!!

Component Mutable is OK.

Choose which one you feel comfortable. :)

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