Skip to content

fix: preserve a multi-character replacement so slug stays idempotent#553

Open
spokodev wants to merge 1 commit into
Trott:mainfrom
spokodev:fix-multichar-replacement-idempotence
Open

fix: preserve a multi-character replacement so slug stays idempotent#553
spokodev wants to merge 1 commit into
Trott:mainfrom
spokodev:fix-multichar-replacement-idempotence

Conversation

@spokodev

Copy link
Copy Markdown

Problem

slug preserves a replacement string that is already present in the input, so that re-slugifying an existing slug is a no-op (idempotent). That preservation only works for a single-character replacement, because it tests char.includes(opts.replacement) against one code unit at a time, which can never match a replacement longer than one character.

As a result a multi-character replacement is treated as disallowed punctuation and stripped:

const slug = require('slug')

slug('foo bar', { replacement: '__' })                              // 'foo__bar'
slug(slug('foo bar', { replacement: '__' }), { replacement: '__' }) // 'foobar'  <- separators lost

A single-character replacement round-trips correctly ('_' → idempotent), so this is an inconsistency rather than intended behavior. The README documents replacement as an arbitrary string with no single-character restriction.

Fix

Detect the replacement with a fixed-length look-ahead and advance past it, mirroring the existing multicharmap handling (i += len - 1). This preserves a replacement of any length while leaving single-character and empty replacements unchanged.

Test

Added an idempotence assertion for a multi-character replacement. It fails before the change and passes after. The full test suite is green and c8 --100 coverage is preserved (100%).

slug already preserves a replacement string that is already present in the
input (so that re-slugifying an existing slug is a no-op), but the check
only worked for a single-character replacement: it tested `char.includes`
on one code unit, which can never match a replacement longer than one
character. A multi-character replacement was therefore stripped as
disallowed punctuation:

  slug('foo bar', { replacement: '__' })            // 'foo__bar'
  slug(slug('foo bar', { replacement: '__' }), ...) // 'foobar'  (separators lost)

Detect the replacement with a fixed-length look-ahead (like the existing
multicharmap handling), advancing past it, so any-length replacement is
preserved and slug is idempotent. Single-character replacements and the
empty replacement 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