Refs Must Have Owner Warning

[TanninOne]

** Reporting a bug

The problem I'm having is in the interplay of several packages but I feel that the only package actually doing something wrong here is react.

Versions:

• react 16.2.0 (also happened with 15.something)
• node.js 7.9 (electron 1.7.10)
• windows 10

** Current behavior

I get the error message from here https://reactjs.org/warnings/refs-must-have-owner.html although none of the three explanations is true.
Actually react is loaded twice but it's the same version and there is little I can do to prevent it.

What happens, as far as I can understand is this:
my application imports react from in two ways

• application -> react
• application -> node_modules/myapi -> react
  both get the same react module from the same path.
  However, the myapi module is installed via yarn link.
  Now on case-insensitive filesystems, yarn link can cause the file path to have different caseing, i.e. my application is in C:/application but the resolved link (which node uses via require) is c:/application/... so the case of the drive letter changes for the api and everything _require_d from the api.

So even though I call "require('react')" both times I end up importing
C:/application/node_modules/react/...
and
c:/application/node_modules/react/...

which is obviously the same thing, yet it means react is loaded twice.

Now the ref that's triggering the error is also not added by me but by react-bootstrap which adds a string-ref.

So my code has no bug (I'm not using different react versions or different case when importing react in my code)
react-bootstrap doesn't do anything wrong (because string refs are still allowed).
node isn't doing anything wrong (since it documents that require isn't guaranteed to always return the instance.)
yarn isn't doing anything wrong (because on a case-insensitive filesystem there is nothing wrong with using different cases for the drive letter between link source and link target.)

react however relies on require always returning the same instance, which it can't.

What is the expected behavior?

React doesn't rely on behavior that is documented to be unreliable.

[gaearon]

I understand your frustration but there's just no way React can coordinate between two different React instances without introducing globals into the environment (which is also bad). String refs require such coordination (which is one of the reasons we encourage people to avoid them). Using two Reacts in the same tree is just not supported for this reason.

If you use webpack, this plugin should help by failing the build whenever this situation occurs. So at least you know when you have this problem.

There is nothing wrong with a module assuming that requires to it won't produce two copies. I'm pretty sure that many of the modules you write also make that assumption. You don't bump into this because you don't extract them to libraries and symlink them, but if you were to do that, you'd also have this issue for any mutable shared state between modules.

[TanninOne]

Using a global is bad but it's infinitely better than this because it's not relying on behaviour that is even documented to not work like this! The require cache doesn't guarantee each module is a singleton and that's a fact.

It doesn't really matter how many modules do this, you don't define Spectre as not-a-bug just because many processors have the same problem.
And no, I don't knowingly write code that makes incorrect assumptions like that and if I accidentally did and someone told me I would understand that it was an error...

I do not use webpack, that's not an option either. And what good does it do me that you discourage using string refs? I don't use string refs in my code, third party modules do. If string refs had been removed from react then you'd have a point, in that case react-bootstrap would be incompatible/bugged. But it's not, so the fault lies with react itself.

[gaearon]

I’m sorry. I can feel you’re very frustrated by this issue. I am too. You’re welcome to propose a solution. I’m not trying to shift the blame; I’m just saying that we have some global state and I’m not aware of the “right” way to store it rather than on a module.

I think that if we solve this by declaring a global variable we’ll just run into other issues. For example some environments might now allow that (e.g. the global object could be frozen). Another issue that may arise is that now several non-conflicting React trees on the page (e.g. two independent third-party widgets that happen to use React and execute in the page context) can now “steal” refs from each other because they share the same global environment. This is really bad because in your case, at least you are in control over the problem, whereas for two third-party widgets it would be impossible to fix on either side.

So I hope you can see this is not a trivial problem. I see your point very well (I’ve written React apps and bumped into this issue) so please don’t think that I’m dismissing this issue. But I’m not aware of a way to solve it. I encourage you to also approach it with an open mind, and not only think about your specific use case, but the other effects of any solution.

With that in mind, if you have other ideas (including adding this to caveats for string refs into the docs), we’d love to know! The docs are open source and you can send a PR too.

[TanninOne]

Well, I'm an old-school c/c++ developer, we were taught globals were bad, whether you call them global or wrap them in a Singleton in an attempt to hide the fact it's a global doesn't really matter.

So intuitively (I haven't looked into the react code) my solution would be: have no global data at all, not in modules, not in the global state but instead pass some kind of "react context object" along.
I.e. User would call "const context = React.makeContext()" and then later "ReactDOM.render(..., ..., context)". Any code that would previously access global data would then access that context instead.
You could then even put a version in the context object and warn if the version of the code being run differs from that of the context object so you'd actually know if data and function doesn't match.
It would also solve problems with having multiple distinct react trees, you could just create multiple context objects and be confident they won't interfere on anything, no matter if they use the same module or not.
This is cleaner, solves problems with global variables and it's better for testing.
Obviously you wouldn't want to call it "context" to avoid confusion with the existing thing, although they are tightly related so you may want to consider just merging them and have a user-controlled and a system part of the context.

And if you're worried about the transition period you can still fall back to using a module-scope variable as a fallback if the context object is undefined.

Now I'll admit I didn't fully understand part of your response. Do you have an example of an environment that doesn't allow adding attributes to global? That sounds like inviting trouble.

Now please don't misunderstand me: If your response is: "A proper solution isn't possible because of the wide range of platforms we need to support." or "we can't change the api at this time" or even "The proper solution is not worth the effort because string refs are being phased out anyway." I can live with that, I know how it is to work under severe limitations.
What bothered me about your first reply was the dismissive tone and essentially this line "There is nothing wrong with a module assuming that requires to it won't produce two copies." triggered me. It may be your least worst alternative given restraints but there obviously is something wrong with it if code that doesn't have any bugs breaks.
Globals (including module variables) and Singletons are always a hack and never the only or even a good solution to anything.

I will also say that it's pretty crappy coding on the part of node.js to base the require cache on resolved file names (instead of, say, the inode number or alternatives that should exist on almost all filesystems)

[gaearon]

So intuitively (I haven't looked into the react code) my solution would be: have no global data at all, not in modules, not in the global state but instead pass some kind of "react context object" along.

The problem with the existing string ref API (which is the one you’d like to see “fixed”) is that it already doesn’t have such an object. The API contract is that <div ref="something" /> magically “remembers” that it should get attached to the currently rendered component, and this is not possible unless we keep track of that currently rendered component.

We fully agree that this was a bad design, and it’s precisely the reason string refs are marked as legacy in the documentation, and will eventually get deprecated and removed. We’ve learned from that mistake, and when we design new APIs, we consider this constraint (avoiding the need for shared mutable state).

If your response is: "A proper solution isn't possible because of the wide range of platforms we need to support." or "we can't change the api at this time" or even "The proper solution is not worth the effort because string refs are being phased out anyway."

I’m just not aware of a “proper” solution.

The one you proposed earlier (using globals) creates other issues as I explained in #12033 (comment):

Another issue that may arise is that now several non-conflicting React trees on the page (e.g. two independent third-party widgets that happen to use React and execute in the page context) can now “steal” refs from each other because they share the same global environment. This is really bad because in your case, at least you are in control over the problem, whereas for two third-party widgets it would be impossible to fix on either side.

So you’ll solve your problem, but other people will create issues about their (new) problems caused by that change.

The other solution you mentioned is to change the API. That’s exactly what we’re doing: string refs are marked as legacy in the docs and we don’t encourage writing new code that relies on them. We can’t deprecate them yet because callback refs aren’t quite as ergonomic, and many developers (including at FB) aren’t ready to rewrite their code that works fine today. We’re considering some solutions that don’t have the pitfalls of string refs but aren’t as annoying to use as callback refs: reactjs/rfcs#17.

Do you see other solutions?

[TanninOne]

The problem with the existing string ref API (which is the one you’d like to see “fixed”) is that it already doesn’t have such an object. The API contract is that

magically “remembers” that it should get attached to the currently rendered component, and this is not possible unless we keep track of that currently rendered component.

See, for this part I just don't know about react internas. It is my understanding that on setting a string-ref react accesses the ReactCurrentOwner global and that since this global is a module variable, if the module generating the div is different from the code that set ReactCurrentOwner, the one in the generating module is null/undefined at the time.
But why we can't opt to have the ReactCurrentOwner in global or a context object that gets passed around automatically I do not know. The context object that is already there does already get passed through the entire tree automatically, right?
So what's stopping you from changing code like this "ReactCurrentOwner.current" to "(context.ReactCurrentOwner || global.ReactCurrentOwner || ReactCurrentOwner).current? Obviously this is super-naive, but this would allow the user to control where to store that "global" without breaking compatibility.

So you’ll solve your problem, but other people will create issues about their (new) problems caused by > that change.

No, I'm not suggesting we break it for anyone else, obviously a solution would have to either support every existing case and then some or be optional.

The other solution you mentioned is to change the API. That’s exactly what we’re doing: string refs are > marked as legacy in the docs and we don’t encourage writing new code that relies on them.

Yeah, sorry, but that's not "changing the API".
Microsoft has deprecated API functionality between Windows 3.0 and Windows 3.11, over 25 years ago and you will find new software being written using that API even today and tutorials that suggest its use...
If you want to get rid of unwanted API then you have to remove it and no matter when you do it and how long you gave users to switch API, there will always be someone unhappy.
Just get it over with...

Do you see other solutions?

Tbh. I like string refs from a user perspective: it's nice to just specify a name and then access the ref through it, it's an api that requires the bare minimum of information from the user, no clutter and overhead.
I just wouldn't store them directly in the owning React object that you then have to track in a global, instead force the user to pass in a "ref holder" to React.createElement.
That ref holder could be passed in automatically when using jsx or through context/a HOC, a user could then even customize how the ref holder works, maybe collect a hierarchical ref tree in a higher tier component.
And if the "ref holder" is undefined when you try to resolve the string ref? Well, then the caller doesn't care about the ref as he wouldn't be able to access it anyway, so you can just skip it, no error.

[gaearon]

for this part I just don't know about react internas

You don't necessarily need to. This follows directly from the React public API. I'll show an example below.

But why we can't opt to have the ReactCurrentOwner in global or a context object that gets passed around automatically I do not know. The context object that is already there does already get passed through the entire tree automatically, right?
So what's stopping you from changing code like this "ReactCurrentOwner.current" to "(context.ReactCurrentOwner || global.ReactCurrentOwner || ReactCurrentOwner).current?

The reason we don't have a similar problem with context is because the parent-child relationship is enough to establish it. So we don't actually need the user to pass it around because we keep track of the tree nesting.

Refs are different. Unlike context, ref "parent" doesn't necessarily match the tree parent.

Consider this example:

function Frame(props) {
  return <div className="Frame">{props.children}</div>
}

class Button extends React.Component {
  render() {
    return <button />
  }
}

class FramedButton extends React.Component {
  render() {
    return (
      <Frame>
        <Button ref='mybutton' />
      </Frame>
    )
  }
}

When rendering, React knows that the resulting rendered tree structure is:

FramedButton
  |- Frame
    |- div
      |- Button
        |- button

If you were to reimplement React from scratch, you could still keep track of the tree structure. The algorithm would be something like:

• take topmost element's type
• create an instance if it's a class / call it as a function if it's a function
• get the result element
• recurse

Note that for a feature like context, you don't actually care when the elements were created. Just the fact that you walk the tree from the top to the bottom and call render functions gives you the "path": FramedButton > Frame > Button. At any point in time you can calculate current context by traversing that path and merging contexts provided by those components in that order.

However, refs are different. From this "path" alone, there is no way you can infer that it's FramedButton that "owns" <Button ref='mybutton' />. This is the essential difference. The only way to know that with the current string ref API is for React (or your hypothetical reimplementation) to write the "currently executing" component somewhere, and for React.createElement() call in the render method to read it—before we exit it and it's too late.

// pseudocode in react-dom
function renderClassComponent(instance) {
  sharedBetweenReactAndReactDOM.currentOwner = instance
  instance.render()
  sharedBetweenReactAndReactDOM.currentOwner = null
}

// pseudocode in react
function createElement(type, config) {
  let ref = config.ref
  if (typeof ref === 'string') {
    let capturedOwner = sharedBetweenReactAndReactDOM.currentOwner
    ref = (inst) => {
      capturedOwner[ref] = inst
    };
  }
  // ...
}

Again, this isn't just an implementation detail, it's the only way you can implement this exact string ref API between two packages. If you want to fix anything about it you'd have to change the API. For example by passing an explicit argument:

class FramedButton extends React.Component {
  render(makeRef) {
    return (
      <Frame>
        <Button ref={makeRef('mybutton')} />
      </Frame>
    )
  }
}

Something like this would solve the issue because then we'd have an explicit "link" between the <Button> element and the FramedButton owner component instance. But again, this is an API change.

Microsoft has deprecated API functionality between Windows 3.0 and Windows 3.11, over 25 years ago and you will find new software being written using that API even today and tutorials that suggest its use...

If you look at React blog you can see we're not afraid of changing the API in every major version to prune deprecations as soon as we have a viable migration path. In this particular case string refs are not deprecated yet, but they will be when we can dedicate more time to this (I expect in the following year). Once we do deprecate them the support will be gone in the next major.

I just wouldn't store them directly in the owning React object that you then have to track in a global, instead force the user to pass in a "ref holder" to React.createElement.

I believe this is pretty much what was proposed in #10581, #11555, and now tracked in reactjs/rfcs#17. It's pretty likely that that's indeed the solution we'll go with. We have more urgent changes to do right now but when we come back to this issue, we'll likely do that.

I hope this helps!

[TanninOne]

Yes, I know the API would need to be changed, the question is just if it could be changed in a way that stays backwards-compatible with existing code, but obviously I can't say what other unwanted effects possible solutions would have.

I understand why the existing context object can't be used to transport the ReactCurrentOwner but not what the problem is with (optionally) storing it in global.

[gaearon]

The problem with storing it in a global is that two different React apps on a single page (that might not even be written by the same team, e.g. two third-party widgets) will start “stealing” refs from each other because they’d use the same global.

[TanninOne]

That's one reason it'd have to be optional.

[gaearon]

How does “optional” work in this case? Who decides on the option? If it’s third-party plugins, then they might not know the option exists. If it’s the app, then it might not have access to React instances inside the third-party plugins. Finally, the plugins may be written in a way that only works with the global (just like you asked for in this issue), so even if the app could “opt them out” of using it they would break.

These kinds of mismatches are exactly why React doesn’t have configuration options and trying to not to rely on shared mutable state in any new APIs. Both are thorny problems with no good solutions.

[TanninOne]

It would have to be the main application obviously. And the implementation shouldn't be too hard, the application creates some storage space in global for react, if that space is there, react uses it, if not, it uses its module variable.
I don't see how this has something to do with third-party plugins, for the time being I'm concerned with reacts own use of a module global that I can't prevent.

[gaearon]

There isn’t always just one “main application” on the page.

A website may use React but also use widgets from third-party websites it has no control over (for example, theoretically, Twitter and Facebook).

If those widgets embed React in them today, they don’t “collide” between themselves because they don’t write and read from shared globals. With your proposal, collisions would be possible and, worst of all, this would be completely out of control of the application author.

I feel like I’m rehashing the same argument over and over again. If it’s not convincing to you that’s fine. I think we can leave the discussion at that.