omniscientjs / immstruct

Immutable data structures with history for top-to-bottom properties in component based libraries like React. Based on Immutable.js
374 stars 21 forks source link

Fix for listListenerMatching in IE8 and potentially other browsers #52

Closed amccloud closed 9 years ago

amccloud commented 9 years ago

Return causes listListenerMatching to return early with undefined. I believe the intention is to just ignore prototype properties.

mikaelbr commented 9 years ago

Very good. This seems to be a logical error, and should affect every browser. Good catch and thanks for fixing. Should probably have test for this causeing https://github.com/amccloud/immstruct/blob/patch-1/src/structure.js#L65-L70 to fail.

Do you already have a working test-case (the code which triggered this error on your side) which you can do as a test?

amccloud commented 9 years ago

I do not have a test case. The issue only happens in IE8 for me.

At a glance my code is doing something very simple.

immstruct = require 'immstruct'
structure = immstruct(messages: [])
structure.reference().cursor()
mikaelbr commented 9 years ago

This might also be due to forEach or are you using a polyfill?

amccloud commented 9 years ago

I'm using a polyfill (es5-shim)

Edit: the key when it fails is forEach even though listeners is []

mikaelbr commented 9 years ago

I merged this even though there are no tests and it's not entirely "closed", but it is an obvious logical error.

Edit: the key when it fails is forEach even though listeners is []

What do you mean by this?