Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Fix array lookup to .includes rather than .has #867

Merged
merged 1 commit into from
Dec 30, 2016

Conversation

charles-toller
Copy link
Contributor

Summary

In the CharacterMeta file, to lookup if a certain CharacterMeta had a style, we were using .has, which looks up keys in an array, while something like 'ITALIC' is stored as a value, so Immutable's .includes function would be more appropriate here.

@hellendag
Copy link

Because DraftInlineStyle is an OrderedSet<string>, it doesn't really matter which one it uses -- both are supported. I would also not be surprised if they have the same performance, or even share the same underlying implementation, though it's unlikely to matter for the typical set size here anyway.

That said, includes does make a bit more sense semantically, so I'm cool with this. :)

@hellendag hellendag closed this Dec 29, 2016
@hellendag hellendag reopened this Dec 29, 2016
@hellendag hellendag merged commit 5540845 into facebookarchive:master Dec 30, 2016
erykpiast pushed a commit to prezly-forks/draft-js that referenced this pull request Mar 17, 2017
ouchxp pushed a commit to ouchxp/draft-js that referenced this pull request Apr 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants