Circular References in React Native

[w0nche0l]

I'm not sure if this is intended behavior that I should post on StackOverflow to find a workaround, or a bug in the very new react native test renderer.

I'm seeing a very strange behavior from my current jest mocks for ListView and RefreshControl. Within the snapshot test, the wrapping component (which is RandomView in the example I wrote below) ListView is rendered multiple times from the RefreshControl's reference.

The resulting snapshot file is here, and you can see that the RandomView is being printed out multiple times, and not being resolved to a Circular reference.

What is the correct solution to this problem? Am I doing something unsupported or just doing something incorrectly?

Mocks:

jest.mock('RefreshControl', () => 'RefreshControl')

jest.mock('ListView', () => {
  const React = require('React')
  const View = require('react-native').View
  return React.createClass({
    statics: {
      DataSource: require.requireActual('ListView').DataSource
    },
    render () {
      // Renders all the elements in the ListView without regards for optimization
      let components = []
      for (let sectionIndex = 0; sectionIndex < this.props.dataSource.sectionIdentities.length; sectionIndex++) {
        if (this.props.renderSectionHeader) {
          components.push(this.props.renderSectionHeader(this.props.dataSource.getSectionHeaderData(sectionIndex), this.props.sectionIdentities[sectionIndex]))
        }
        if (this.props.renderRow) {
          for (let rowIndex = 0; rowIndex < this.props.dataSource.rowIdentities[sectionIndex].length; rowIndex++) {
            components.push(this.props.renderRow(this.props.dataSource.getRowData(sectionIndex, rowIndex)))
          }
        }
      }

      // passes the props to the rendered view
      return (<View {...this.props}>{components}</View>)
    }
  })
})

Component

import React, {PropTypes} from 'react'
import {
  ListView,
  RefreshControl,
  View
} from 'react-native'

var RandomView = React.createClass({
  propTypes: {
    list: PropTypes.array.isRequired
  },

  getDataSource: function (list) {
    return new ListView.DataSource({rowHasChanged: (r1, r2) => r1 !== r2})
  },

  render: function () {
    return (
      <ListView
        enableEmptySections
        dataSource={this.getDataSource(this.props.list)}
        renderRow={this._renderRow}
        refreshControl={
          <RefreshControl />
        }
      />
    )
  },

  _renderRow: function (rowData) {
    return (
      <View />
    )
  }
})

export default RandomView

Test

import 'react-native'
import React from 'react'
import RandomView from '../RandomView'

// Note: test renderer must be required after react-native.
import renderer from 'react-test-renderer'

describe('<RandomView />', () => {
  it('renders correctly', () => {
    const tree = renderer.create(
      <RandomView list={[{a: 1}, {a: 2}]} />
    ).toJSON()
    expect(tree).toMatchSnapshot()
  })
})

[cpojer]

I'm not sure I follow, RandomView is only rendered once?

[w0nche0l]

Sorry, I wasn't super clear as to where the error occurs.
So if you look at line 3, 84, 112, 156, 185, 228, they all seem to be the same component with the same props (children, dataSource, enableEmptySections). So I suppose it is that the ListView is getting rendered multiple times.

[cpojer]

I see. Yeah, we need to stop trying to render React elements.

cc @gaearon – we talked about this before but then I got a bit confused about what exactly to do in the pretty formatter.

[gaearon]

The problem is that pretty-format treats children prop specially and assumes that only children would be elements. In reality any prop could contain elements, or objects/arrays containing elements.

The proper way to check if something is an element is to verify it has a $$typeof property with the value of Symbol(react.element).

[gaearon]

So, to be clear, this special handling for React elements should be here.
Any object could be a React element, or contain objects that are React elements.

For example:

<MyComponent props={{
  theme: {
    message: <BlueMessage /> /* React element inside an object */
  }
}} />

[cpojer]

But it shouldn't be <BlueMessage />, right? It should render as something like [react.element BlueMessage] or some other representation, no?

[gaearon]

It should be <BlueMessage />.

<BlueMessage /> is equivalent to React.createElement(BlueMessage) and equivalent to { type: BlueMessage, $$typeof: Symbol('react.element'), props: {} }.

These are three ways to write the same thing, and JSX is the readable one.

[cpojer]

I started implementing this in jamiebuilds/pretty-format#28 and it got confusing pretty quickly because I was suddenly dealing with elements, instances and test renderer instances and I ended up printing quite deeply and it wasn't really useful.

[gaearon]

I don't quite understand what you were trying to do in that PR. I think maybe you got confused over some part of it. I can implement this tomorrow if you'd like.

[gaearon]

I do not mean that you need to iterate over props and find "hidden children" or something like this. I mean that you need to yank out the logic for printing JSX into the generic printObject and then you shouldn't have this problem at all.

If not I would appreciate a test case showing the problem so I can work against it.

[cpojer]

I was simply trying to print React elements using the existing react test renderer plugin – including props and its children. Note that we don't need to change pretty-format itself but only the React plugin. Plugins in pretty-format come with a test function and every object gets tested for a match. If you patch the diff you'll find that it doesn't quite work as intended, I think the problem was nested React children. I'd be happy to see a PR with the proper implementation, though – we can chat tomorrow and I can point out the issues I found. In the meantime I'll close this as a duplicate of #1429.

[gaearon]

I think it comes down to what you want to capture in the snapshot.

With DOM, we just capture the result DOM tree. So it should be relatively safe to completely throw away any elements (or replace them with some dummy placeholder) because by this point React won't interpret them anyway (and will warn you about extra meaningless props). In the future however we might allow some props on DOM to accept elements so we probably shouldn't lock ourselves out of supporting that.

In native it seems like we render down to View. However in RN we don't currently warn about meaningless props (such as refreshControl which should not have been passed to View in the first place). If RN had parity with DOM and warned about these it would not be a problem but I don't know if we plan to do that (cc @jimfb).

I still think pretty printing elements is the right thing to do. When the tree gets deep it usually just means that the component you're snapshotting passes unnecessary props down. For example if ListView didn't blindly forward its props to View they wouldn't end up in the snapshot. So it's the component that needs to be fixed to prevent deep pretty prints, not pretty printer.