Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support for iterable structures within state #79

Merged
merged 5 commits into from
Aug 23, 2015

Conversation

danielkcz
Copy link
Contributor

Alright, I finally needed this so badly that I had to do it :) I haven't tested all available structures, but here is view of my Immutable.List of Immutable.Record instances within state.

redux-devtools-iterable

Fixes #51, #66, #70

@danielkcz danielkcz changed the title support for state with iterable structures support for iterable structures within state Aug 20, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 20, 2015

Good stuff, thank you! Anybody else wants to test drive this?

@quirinpa
Copy link

I think i do eventually at least

@bnoon
Copy link

bnoon commented Aug 21, 2015

With a couple of changes, this will work for Map() as well, so I am closing #66 in favor of this more general solution.

@bnoon bnoon mentioned this pull request Aug 21, 2015
} else {
count = data.length;
}
this.itemString = count + ' entr' + (count !== 1 ? 'ies' : 'y');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to the following

      if (typeof data.count === 'function') {
        count = data.count();
      } else if (typeof data.size !== 'undefined') {
        count = data.size;
      } else {
        count = data.length;
      }

To handle the length of a Map()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree. I had the size there before, but then I've noticed that MoriJS has only count method.

I am thinking if it wouldn't be better to use iterator even here to count entries. That would be unified way. I would like to avoid adding another property/method in a future if some other data structure implementing iterator would use different name for this. Some library could even have a like size being method or count being property.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any conclusion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've change it. It uses the size property as it's the most common (only mori doesn't have it), otherwise it counts entries through the iterator. I am not using length because it's deprecated in Immutable anyway and others doesn't have it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another small change to use Number.isSafeInteger for the size property. It should be safer way in case some library has size as a function.

@gaearon
Copy link
Contributor

gaearon commented Aug 23, 2015

Out in 1.1.0, let me know how it goes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants