deepDiffer causing lots of Array iteration logspew

[ide]

See:

Your code is broken! Do not iterate over arrays with for...in. See https://fburl.com/31944000 for more information.

    at Array.Object.defineProperty.Object.defineProperty.get [as __ARRAY_ENUMERATION_GUARD__] (Array.prototype.es6.js:26)
    at Object.deepDiffer [as diff] (deepDiffer.js:39)
    at diffRawProperties (diffRawProperties.js:54)
    at ReactIOSNativeComponent.Mixin.computeUpdatedProperties (ReactIOSNativeComponent.js:150)
    at ReactIOSNativeComponent.Mixin.receiveComponent (ReactIOSNativeComponent.js:189)
    at Object.ReactReconciler.receiveComponent (ReactReconciler.js:97)
    at ReactCompositeComponentMixin._updateRenderedComponent (ReactCompositeComponent.js:726)
    at ReactCompositeComponentMixin._performComponentUpdate (ReactCompositeComponent.js:705)
    at ReactCompositeComponentMixin.updateComponent (ReactCompositeComponent.js:625)
    at ReactPerf.measure.wrapper [as updateComponent] (ReactPerf.js:70)

[dvbportal]

For me it is unclear why for..in should be considered broken. (The link 'https://fburl.com/31944000' is gone)

I also refactored some of those loops into .forEach(function() {..., but there are some 'this'-related side effects.

[sophiebits]

Sorry, the link goes to an FB-internal page:

You can pass this as a second argument to forEach or use an arrow function to preserve the context.

[dvbportal]

Thanks Ben, for clearing that up. I see now, that it is a good idea to give that warning.

[josebalius]

@spicyj @dvbportal so guys is this still a bug or is there a proper way to do this? I am having the same error with ListView.

[sophiebits]

Still an issue unfortunately. Should be fixed with the next sync though.

[soheil-zz]

+1

[romanonthego]

+1
..and any updates? it's impossible to use many of libraries right now (baobab and typology for example) without monkey patching it.

[sahrens]

I landed a fix internally that should get sync'd out soon.

On Apr 23, 2015, at 11:54 PM, Roman Dubinin [email protected] wrote:

+1
..and any updates? it's impossible to use many of libraries right now (baobab and typology for example) without monkey patching it.

—
Reply to this email directly or view it on GitHub.

[ide]

@sahrens is your fix to make the polyfilled methods non-enumerable? That's the cleanest solution I had in mind since RN is used only in ES5+ environments.

[sahrens]

My fix just checks Array.isArray in deepDiffer and does the appropriate type of iteration.

On Apr 24, 2015, at 12:15 AM, James Ide [email protected] wrote:

@sahrens is your fix to make the polyfilled methods non-enumerable? That's the cleanest solution I had in mind since RN is used only in ES5+ environments.

—
Reply to this email directly or view it on GitHub.

[ide]

Got it - that fix seems right too. For the Array.prototype polyfill I think we want to use Object.defineProperty to add the polyfilled methods as non-enumerable properties. I imagine this was not done consciously for www so is it possible for www and RN to have different copies of the polyfill?

[vjeux]

It was done consciously for www because we want to have the same behavior with IE8 that doesn't support Object.defineProperty. If we were to use Object.defineProperty, we would break IE8 in a lot of terrible ways as most developers are not testing against IE8. But, you are right, it doesn't seem like a good way to polyfill it in React Native.

[sophiebits]

Fixed in ab1efbd.