Skip to content

fix: treat null mappings from extend() as character removal#207

Open
abhu85 wants to merge 1 commit into
simov:masterfrom
abhu85:fix/177-extend-null-removal
Open

fix: treat null mappings from extend() as character removal#207
abhu85 wants to merge 1 commit into
simov:masterfrom
abhu85:fix/177-extend-null-removal

Conversation

@abhu85

@abhu85 abhu85 commented Jun 7, 2026

Copy link
Copy Markdown

Fixes #177

Problem

extend({ '%': null }) was used in 1.6.5 to make a character drop out of the slug (e.g. "100%""100"). Since 1.6.6 the same call throws:

TypeError: Cannot read properties of null (reading 'replace')

In the reduce that builds the slug, the lookup resolves to null:

var appendChar = locale[ch];
if (appendChar === undefined) appendChar = charMap[ch]; // -> null (from extend)
if (appendChar === undefined) appendChar = ch;          // null !== undefined, so stays null
...
return result + appendChar.replace(/* ... */)           // null.replace(...) throws

Fix

Coerce a null mapping to an empty string, so the character is removed — restoring the pre-1.6.6 behavior and preventing the crash:

if (appendChar === null) appendChar = '';

undefined mappings are unchanged (they still fall back to the original character), and string/empty-string mappings are unaffected.

Tests

Added a regression test (mirrors the existing extend() tests, with the usual require.cache reset):

slugify.extend({ '|': null, '%': null, $: null })
t.equal(slugify('100%'), '100')
t.equal(slugify('a|b$c'), 'abc')

Verified it fails without the fix (the reported TypeError) and passes with it. Full suite: 46 passing.

extend({ '%': null }) worked in 1.6.5 to drop a character (e.g. "100%"
-> "100"), but since 1.6.6 it throws "TypeError: Cannot read properties
of null (reading 'replace')": the charMap lookup yields null, the two
`=== undefined` fallbacks don't catch it, and null.replace() throws.

Coerce a null mapping to an empty string so the character is removed,
restoring the earlier behavior and preventing the crash.

Fixes simov#177
@Trott

Trott commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

This change seems OK to me, but also as an FYI: If you want to switch to the slug module, which would likely be a drop-in replacement (if you're using ESM), it (1) doesn't do magic with special characters like % so it will do what you are trying to achieve with extend({'%': null}) by default, and (2) handles extend({'%': null}) in the way you expect.

import slug from 'slug';
import slugify from 'slugify';

// Default behavior of slug() is already what you want.
console.log(slug('100%')); // 100
console.log(slugify('100%')); // 100percent

// Extend works the same way for both libraries.
slug.extend({ '%': ' percent' })
slugify.extend({ '%': ' percent' })
console.log(slug('100%')); // 100-percent
console.log(slugify('100%')); // 100-percent

// Null currently throws in slugify, but works as expected in slug.
slug.extend({ '%': null })
slugify.extend({ '%': null })

console.log(slug('100%')); // 100
console.log(slugify('100%')); // Throws TypeError

All that said, I like this code change. For selfish reasons, I'm in favor of anything that brings slugify() more in alignment with slug().

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.

As of 1.6.6, extending a character with "null" will break

2 participants