Skip to content

Expose refs on DOM Components as Direct Handles to the actual DOM Node #3223

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

Closed
sebmarkbage opened this issue Feb 21, 2015 · 8 comments
Closed

Comments

@sebmarkbage
Copy link
Collaborator

My mental model is that we have a polyfill layer (ReactDOMInput) to support what the browser would support, and will support in the future once these ideas propagate through the standards. The fact that it is currently implemented as a wrapper should be an unobservable implementation detail, and perhaps it shouldn't be implemented as a wrapper.

As an upgrade path we would need to monkey patch existing legacy methods (props, setProps, getDOMNode) onto it as an upgrade path. These would work but immediately warn when used.

setProps will be replaced by some other imperative layer, or simply multiple calls to React.render.

component.getDOMNode is being replaced by React.findDOMNode(component) which can take either an arbitrary component or a DOM node directly. So you can use it on something that you're unsure about. In most cases, you can simply get rid of the call all together and use the DOM ref directly.

@sebmarkbage sebmarkbage mentioned this issue Feb 21, 2015
25 tasks
@syranide
Copy link
Contributor

My mental model is that we have a polyfill layer ( ReactDOMInput ) to support what the browser would support, and will support in the future once these ideas propagate through the standards. The fact that it is currently implemented as a wrapper should be an unobservable implementation detail, and perhaps it shouldn't be implemented as a wrapper.

I agree about "should be an unobservable implementation detail". But how would you accomplish "perhaps it shouldn't be implemented as a wrapper."?

Anyway, it seems to me that direct interaction with the underlying frontend objects is something we want users to avoid? Instead favoring a reduced API bridge (when necessary) which would just fulfill the needs of the users and one that could also be reimplemented/proxied by community wrappers intended to fix/improve/extend/emulate native components for compatibility. This reduced API bridge could also provide additional benefits such as caching, batching or alternative APIs suitable for the React environment. Accessing the underlying frontend object would then be the "last resort".

The idea of returning the actual DOM node instead of a component for a ref is really interesting... the more I think about the more directions/reasons for it appear to me so I can't really boil down my questions/concerns to something sensible so I'll just trust you ;)

setProps will be replaced by some other imperative layer, or simply multiple calls to React.render.

That wouldn't work for toElement though?

@syranide
Copy link
Contributor

@sebmarkbage This wouldn't work with e.g. #2242 I think? There's no universal guarantee the underlying DOM node lives as long as the owning component, unless polyfilled DOM components are exposed as such, but that doesn't seem to be your plan? EDIT: But even then, we're forcing that guarantee to hold for all frontend implementations unless there's a way to force refresh refs?

@sebmarkbage
Copy link
Collaborator Author

Refs gets attached/reattached all the time: E.g:

<div key={Math.random()}><div ref="myDiv" /></div>

We respond by firing the ref callbacks and updating this.refs[key]. The only difference is if you stash the ref away in your own state or closure somewhere.

Even then, it seems like this is only a problem if you have a ref on an input whose type changes and only in IE8 (which is quickly dying). That seems like a very uncommon edge case.

@syranide
Copy link
Contributor

@sebmarkbage

Even then, it seems like this is only a problem if you have a ref on an input whose type changes and only in IE8 (which is quickly dying). That seems like a very uncommon edge case.

But are we sure this holds for every frontend out there? I agree input is kind of special here, but could one not imagine something similar for other frontends too? But then again, being able to refresh refs would solve that if it ever came to that.

@sebmarkbage
Copy link
Collaborator Author

ZOMG! So exciting.

@oztune
Copy link

oztune commented Jul 28, 2015

@sebmarkbage With this new change, how would we reference an actual component instance? For example, how would I implement this hypothetical Dock component (that acts like the OSX dock) in 0.14.x ?

class DockItem {
    bounce () {
        //... bounces the dock item
    }
    render () {
        return ...
    }
}

class Dock {
    componentDidMount () {
        // Listen to an event from the app, then bounce its
        // dock icon.
        system.getApp('XCode').addEventListener('alert', e => {
            this.xcode.bounce();
        });
    }

    return (
        <Dock>
            <DockItem ref="xcode" />
            <DockItem ref="terminal" />
            <DockItem ref="photoshop" />
        </Dock>
    )
}

@sophiebits
Copy link
Collaborator

Those work the same; only low-level DOM components (e.g., <div>) are affected.

@oztune
Copy link

oztune commented Jul 28, 2015

@spicyj Ah ok, that's great to know! Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants