Expose key for helper libraries

[natew]

Do you want to request a feature or report a bug?
Feature

What is the current behavior?
You can't access this.props.key / this.key

What is the expected behavior?
We have a use case where we hydrate/dehydrate certain components for an HMR plugin we've developed internally.

But, when HMR'ing a file, we often need to access things within loops, and rehydrate the right data.

Right now, we have to add a secondary key to everything, but this is only for our development tool.

If we had access to keys, this would be trivial to implement. It's also impossible to implement with a babel compiler trick, because we need to detect these components/loops at runtime, and we can't just add a secondKey prop everywhere because we'd have a mess in our props.

It seems like it's rather end-user-hostile to not at least give a backdoor into accessing keys, so my proposal, for all the weird edge case uses like our own here, would be to expose:

React.getKey()

Which would take a component instance and return the key for it.

There could be appropriate warnings around it in documentation and elsewhere.

[gaearon]

Is it just state you want to hydrate / dehydrate?
Sounds like a part of #4595.

[natew]

Actually has to do with the state of stores that are attached to a given child view. The HMR hits the parent view, which then re-renders the child. The child has a decorator that instantiates and attaches any stores necessary.

The stores then need some sort of key to know which instance they should attach. For now we are attaching a separate "storeKey", which is pretty hacky, and causes really strange bugs because we can't really detect if it should be there, because child views have no idea if they are attached to a key or a singleton (usually they should be attached to a key).

[gaearon]

Can you create a very reduced case in JS so I better understand what you're trying to do?

[natew]

Ok, tried to make this halfway legible, it's extracted and simplified quite a bit. Don't think it compiles, but lmk if it makes sense:

// attach.js

let previousStores = {}

export default (fileId, provided, child) => {
  if (module && module.hot) {
    const { stores } = module.hot.data
    previousStores[module.id] = stores[fileId]
  }

  return class StoreProvider extends Component {
    state = {
      stores: {}
    }

    componentDidMount() {
      const storesOld = {}

      // hmr
      if (module && module.hot) {
        // restore
        if (previousStores[module.id]) {
          const previous = previousStores[module.id]

          Object.assign(storesOld, previous._isKeyed_ ?
            previous[this.props.storeKey]
            previous
          )
        }
        // save
        module.hot.dispose(data => {
          data.stores = {}

          const storeNames = Object.keys(provided)

          for (const name of storeNames) {
            const store = this.state.stores[name]

            // allow restore of storeKey'ed stores
            if (this.props.storeKey) {
              data.stores[name] = data.stores[name] || { _isKeyed_: true }
              data.stores[name][this.props.storeKey] = store
            }
            else {
              data.stores[name] = store
            }
          }
        })
      }

      // create stores
      storeName.forEach(name => {
        let restoreStore

        if (storesOld[name]) {
          // restore old instance
          restoreStore = storesOld[name]
        }
        else {
          // create new
          restoreStore = new stores[name](this.props)
        }

        this.setState({
          stores: { ...this.state.stores, [name]: restoreStore }
        })
      })
    }

    render() {
      // pass this.state.stores to child
    }
  }
}

// child.js
import attach from './attach.js'

class Store {
  constructor({ index }) {
    this.index = index
  }
}

let Child = attach(
  1,
  { hello: Store },
  ({ stores }) => <div>{stores.hello.index}</div>
)

let Parent = () => <div>
  {[1,2,3].map(i => <Child key={i} storeKey={i} index={i} />}
</div>

[gaearon]

I'm not sure I see how React's key fits there..

[gaearon]

Key is not globally unique, right? How do you solve that problem?

[AoDev]

Hi, I'd like to add that I also wish to have the key exposed. Mainly to be able to unit test components key. Since a wrong / missing key can break the UI.

It's especially useful where we have some generated key based on props. It happens mainly in lists where you might have items that don't have a unique id to use.

[0x24a537r9]

Similarly to @AoDev I'm interested in the for use in testing. I'm in the process of building a module that renders two trees using react-test-renderer, then diffs them so that you can take snapshots of their differences, e.g.:

it('shows diffs for single components with equal and different props', () => {
  expect(
    diffReact(<View diff1="a" equal1="b" />, <View diff1="c" equal1="b" />),
  ).toMatchSnapshot();
});

Yields a diff like:
    <VIEW
  -   diff1="a"
  +   diff1="c"
      …=…
    />

The idea is that rather than snapshotting an entire rendered tree, we can snapshot only the changes between two states (e.g. a button and the same button when disabled). This also helps assert something that is otherwise very difficult to test--that your component tree is structured to minimize mutations (for which keys are critical). See jestjs/jest#2202 (comment) for more context.

Keys are critical for performing such virtual-dom diffs, and I'm not sure what we gain by not exposing them (or at least having the option to expose them).

[gaearon]

Hi, I'd like to add that I also wish to have the key exposed. Mainly to be able to unit test components key. Since a wrong / missing key can break the UI.

The key is exposed where it needs to be (on the element). If you find it useful to unit test a component rendering a list, get its render output (e.g. with a shallow renderer) and then look at .key of elements in the array.

I'm in the process of building a module that renders two trees using react-test-renderer [...] I'm not sure what we gain by not exposing them (or at least having the option to expose them).

This is a separate issue that sounds unrelated to this issue. What you’re proposing is specific to test renderer, and I think we could easily expose keys there. Would you like to file a new issue about this or send a PR?

[sophiebits]

Hmm, I'm not opposed per se to exposing keys in the test renderer, though I would default to the assumption that no renderer, including the test renderer, should even be aware of keys as they are an application-level concept. I guess the test renderer is already special since we're planning to expose composites in tree traversal methods.

@0x24a537r9 I don't think I fully understand your use case though. Why do you need the keys in order to output a textual diff of the trees?

[0x24a537r9]

@gaearon Oh geez, my bad. I see what happened. I had a few different tabs open with requests to expose keys here in React and in the React test renderer repo also. I actually just wanted react-test-renderer to expose keys--not React. I think after coming back from lunch I just saw the title for this issue and mistook it for the one for the renderer! Sorry for the confusion.

[gaearon]

I don't think I understand how the original request helps solve the general problem of hot reloading. Just knowing the key is not enough. You need to know the whole "key path" I think. If you're developing a system that has a built-in notion of "stores" then it's already bigger than React, and you might as well attach your own implicit keys or override createElement to attach them for you.

The bigger issue related to hot reloading is, I think, #4595, so let's close this one.

[natew]

I think this is still valid.

Since using a local-only store has no global state tree, if HMR wants to re-hydrate it needs a way to determine what exactly store it should be looking up in the new component that is being rendered.

I still think having access to the "tree" structure in React (a unique and stable key that represents the path to the current component) would give us the right solution. It would also help with #4595 in a sense as you could then use the tree paths React gives you to externalize your state tree somewhere. But it would be generic and not tied to state/setState.

Our current solution is hacky and buggy because it relies on DOM structure, which changes based on state. I'd really love some way to access this, as it would (and would have almost a year ago) make my day to day a hell of a lot more efficient.

[markerikson]

Waking up an ancient thread:

Right now React-Redux v5 and v7 depend on using a custom Subscription class in order to enforce a top-down tiered/cascading update pattern. This is necessary because of our need to guarantee that connect wrapper components only run their subscription callbacks once the parents have re-rendered, so the subscription callback has the latest wrapper props available.

In v5, the wrapper component overrode this.context.subscription. In v7, we do const overriddenContextValue = {...contextValue, subscription}, and then re-render the given ReactReduxContext.Provider instance.

This works okay for connect as a HOC, but seems like it would be a blocking issue for some kind of a useRedux() hook, as hooks can't put anything into context.

If we had some kind of a "current component keypath" available, then I could imagine some kind of custom subscription management logic in the root of the tree (sorta similar to https://www.npmjs.com/package/react-state-tree ) that would reorder subscription entries in a tree form and allow us to manage that update cascading behavior from the root, without having to always override a context value with a nested subscription instance.

I'm not saying this is necessarily a good idea, but it also seems that something like that might be relevant for us eventually implementing a hooks API (at least if we want to maintain our current behavior around store subscriptions and props handling).