Fix implementation to avoid Object.prototype mutation, + perf improvement#47
Conversation
…fy"" This reverts commit bfb2e78.
- also bumped the version a minor, as new functionality is added
|
@omnidan also removed an unnecessary nested loop, as the |
|
Awesome; I just started a PR, but I'll cancel that one 😁 |
| */ | ||
| Emoji.unemojify = function unemojify(str) { | ||
| if (!str) return ''; | ||
| var words = toArray(str); |
There was a problem hiding this comment.
var words = Array.from(str);//cc @paulmillr
There was a problem hiding this comment.
Indeed, this works properly with Emojis:
> const emoji = '😍🌸'
undefined
> require('lodash').toArray(emoji) // lodash version, understands unicode
[ '😍', '🌸' ]
> [].slice.call(emoji) // slice version, doesn't get unicode chars
[ '�', '�', '�', '�' ]
> Array.from(emoji) // yeey! no need for the dependency, understands unicode
[ '😍', '🌸' ]There was a problem hiding this comment.
Does not work for complex emojis. Try for something like 👩👩👧👧. Thats why I used the lodash library.
There was a problem hiding this comment.
Aah 😢. Lodash maintainers did a good job with their implementation then!
bbrzoska
left a comment
There was a problem hiding this comment.
Better to use the native Array.from, see my inline comment.
| */ | ||
| Emoji.unemojify = function unemojify(str) { | ||
| if (!str) return ''; | ||
| var words = toArray(str); |
There was a problem hiding this comment.
Indeed, this works properly with Emojis:
> const emoji = '😍🌸'
undefined
> require('lodash').toArray(emoji) // lodash version, understands unicode
[ '😍', '🌸' ]
> [].slice.call(emoji) // slice version, doesn't get unicode chars
[ '�', '�', '�', '�' ]
> Array.from(emoji) // yeey! no need for the dependency, understands unicode
[ '😍', '🌸' ]|
thank you so much for all the effort @timruffles ! this PR looks good and I'll merge it! |
|
published |
Uh oh!
There was an error while loading. Please reload this page.