React: A function that is also an iterable is treated inconsistently

Created on 30 Oct 2017 · 11Comments · Source: facebook/react

Do you want to request a feature or report a bug?
bug (I think)

What is the current behavior?
Here's a codesandbox. Or just plop this in an index.html:

<body>
<div id="root"></div>
<script src="https://unpkg.com/[email protected]/umd/react.development.js"></script>
<script src="https://unpkg.com/[email protected]/umd/react-dom.development.js"></script>
<script src="https://unpkg.com/[email protected]/babel.js"></script>
<script type="text/babel">
function makeRenderable(fn, valueGetter) {
  const ITERATOR_SYMBOL =
    (typeof Symbol === 'function' && Symbol.iterator) || '@@iterator'
  function iterator() {
    let timesCalled = 0
    return {
      next() {
        const done = timesCalled++ > 0
        return {done, value: done ? undefined : valueGetter()}
      },
    }
  }
  fn[ITERATOR_SYMBOL] = iterator
  return fn
}

const renderableFunction = makeRenderable(function(a, b) {
  // kinda irrelevant what goes on in here...
  return a + b
}, () => 'I am the rendered version of the function')

function Working() {
  return (
    <div>
      This works: <div>{renderableFunction}</div>
    </div>
  )
}

function NotWorking() {
  return <div>This does not work: {renderableFunction}</div>
}

ReactDOM.render(
  <div>
    <Working />
    <hr />
    <NotWorking />
  </div>,
  document.getElementById('root'),
)
</script>
</body>

What is the expected behavior?

I expect that things will render the same regardless of whether they're an only child or a member of an array of children.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Yes, it worked in React@15.

Suggested Solution

I'm fairly confident that the issue is in the array of children case, the getIteratorFn call is within a typeof newChild === 'object' && newChild !== null if statement, whereas the original reconcileChildFibers puts the getIteratorFn call outside it.

If I'm not mistaken, a simple fix would be to move this curly brace a few lines up :wink:

I'm happy to do so complete with a test given a little direction (like, where the test should go). Thanks!

Reconciler medium Bug good first issue (taken)

Source

kentcdodds

👍1

All 11 comments

We could move it out, but we should make sure that:

We still should throw if something has typeof object but is neither of supported object types
We still should not throw if it has an unsupported type but isn't an object
We should not call getIteratorFn unnecessarily (e.g. on an element)

Maybe you could just duplicate this code path below. Or figure out some other smart way to do it.

The test should go in the same place existing iterable tests currently go. You can search for them in packages/**/*-test.js with @@iterator definitions. I see one in ReactChildReconciler-test.js, ReactMultiChild-test.js, ReactMultiChildReconcile-test.j.

Find the one that's most similar and put it next to it :-)

gaearon on 31 Oct 2017

I've got some time today so I'll start working on fixing this. Thanks @gaearon!

kentcdodds on 21 Nov 2017

We decided to do the opposite: the fix would be to completely disable this patterns since it was never intentionally supported. Iterable functions should be treated as functions, not iterables.

gaearon on 5 Jan 2018

So the action item here is to send a PR that completely disables this functionality.

gaearon on 5 Jan 2018

👍1

Hey I am new to Opensource ,Can i be of some help?

counterJ on 11 Jan 2018

❤1

Sure. It's probably not the easiest issue, but if you'd like to give it a try, please go ahead!

gaearon on 11 Jan 2018

Are you still working on this blacktrident? If not I may give it a go

moe091 on 4 Feb 2018

What's the status on this one? Do we wish to remove the support for reconciling iterables? Would this include changes to the reconcileChildrenIterator function?

raunofreiberg on 16 Aug 2018

@raunofreiberg We only want to remove support for treating functions as iterables. Meaning literally function values that have iterator attached to them. This worked but accidentally.

Feel free to PR.

gaearon on 16 Aug 2018

Gotcha. I'll try and start working on this - thanks!

raunofreiberg on 16 Aug 2018

👍1

Fixed in https://github.com/facebook/react/pull/13416.

gaearon on 16 Aug 2018

Was this page helpful?

0 / 5 - 0 ratings