Skip to content

fix: strip and replace emoji that have a skin tone modifier - #321

Open
spokodev wants to merge 1 commit into
omnidan:mainfrom
spokodev:fix-strip-replace-skin-tone
Open

fix: strip and replace emoji that have a skin tone modifier#321
spokodev wants to merge 1 commit into
omnidan:mainfrom
spokodev:fix-strip-replace-skin-tone

Conversation

@spokodev

Copy link
Copy Markdown

Problem

strip() and replace() leave skin-tone-modified emoji in place, even though which() and unemojify() recognise them:

const emoji = require('node-emoji')

emoji.strip('X馃憤馃徑Y')            // 'X馃憤馃徑Y'  (expected 'XY')
emoji.replace('馃憤馃徑', () => 'Z')  // '馃憤馃徑'    (expected 'Z')

emoji.which('馃憤馃徑')              // '+1'    (recognised)
emoji.unemojify('馃憤馃徑')          // ':+1:'  (recognised)
emoji.strip('X馃憤Y')             // 'XY'    (plain emoji works)

char-regex already tokenises 馃憤馃徑 as one character, so the gap is purely in the lookup. The README documents strip as "Remove all of the emojis from a string" and replace as "Replace the emojis in a string", and a skin-toned emoji is unambiguously an emoji (the lib's own which returns +1 for it), so leaving it in place is a contract/consistency violation.

Cause

replace() looks the character up with findByCode(character) on the raw character, while which() first normalises the skin tone:

// which.ts
const result = findByCode(skinTone(emoji, 'none'))
// replace.ts (before)
const found = findByCode(character)

strip() delegates to replace(), so both are affected.

Fix

Apply the same skin-tone normalisation in replace() that which() already uses. skin-tone is already a dependency, and the base shortcode table contains no skin-tone scalars, so plain emoji and existing matches are unaffected.

Tests

Added skin-tone cases to strip.test.ts and replace.test.ts. They fail on main and pass with the fix. Full suite green (74 tests), including the existing complex-ZWJ, flag, and preserveSpaces cases.

`strip()` and `replace()` left skin-tone-modified emoji in place, even
though `which()` and `unemojify()` recognise them:

    strip('X馃憤馃徑Y')            // 'X馃憤馃徑Y'  (expected 'XY')
    replace('馃憤馃徑', () => 'Z')  // '馃憤馃徑'    (expected 'Z')
    which('馃憤馃徑')              // '+1'    (recognised)

`replace()` looked the character up with `findByCode(character)` on the raw
character, while `which()` first normalises the skin tone with
`findByCode(skinTone(emoji, 'none'))`. Apply the same normalisation in
`replace()` (which `strip()` delegates to). `skin-tone` is already a
dependency, and the base table contains no skin-tone scalars, so plain
emoji are unaffected.
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.

1 participant