Fix slugifyWithCounter() returning the same slug twice

index.js

@@ -125,6 +125,7 @@ export default function slugify(string, options) {
 
 export function slugifyWithCounter() {
 	const occurrences = new Map();
+	const returned = new Set();
 
 	const countable = (string, options) => {
 		string = slugify(string, options);
@@ -137,16 +138,24 @@ export function slugifyWithCounter() {
 		const numberless = occurrences.get(removeCounterSuffix(stringLower)) || 0;
 		const counter = occurrences.get(stringLower);
 		occurrences.set(stringLower, typeof counter === 'number' ? counter + 1 : 1);
-		const newCounter = occurrences.get(stringLower) || 2;
-		if (newCounter >= 2 || numberless > 2) {
-			string = `${string}-${newCounter}`;
+		let newCounter = occurrences.get(stringLower) || 2;
+		let result = newCounter >= 2 || numberless > 2 ? `${string}-${newCounter}` : string;
+
+		// The counter is keyed on the incoming slug, so it cannot see a slug that was previously handed out by appending a counter to a *different* input. Without this loop, `foo`, `foo`, `foo 2` would return `foo-2` twice. Keep bumping the counter until the result is one that has not been returned before.
+		while (returned.has(result.toLowerCase())) {
+			newCounter += 1;
+			occurrences.set(stringLower, newCounter);
+			result = `${string}-${newCounter}`;
 		}
 
-		return string;
+		returned.add(result.toLowerCase());
+
+		return result;
 	};
 
 	countable.reset = () => {
 		occurrences.clear();
+		returned.clear();
 	};
 
 	return countable;

test.js

@@ -262,6 +262,36 @@ test('counter', t => {
 	t.is(slugify2(''), '');
 });
 
+test('slugifyWithCounter() never returns the same slug twice', t => {
+	// A slug produced by appending a counter to one input can collide with the plain slug of a different input, as the counter is keyed on the incoming slug only.
+	const slugify = slugifyWithCounter();
+	t.is(slugify('foo'), 'foo');
+	t.is(slugify('foo'), 'foo-2');
+	t.is(slugify('foo 2'), 'foo-2-2');
+
+	slugify.reset();
+
+	// The same collision in the other order.
+	t.is(slugify('foo'), 'foo');
+	t.is(slugify('foo 2'), 'foo-2');
+	t.is(slugify('foo'), 'foo-3');
+
+	slugify.reset();
+
+	// The bump has to keep going while the next candidate is also taken.
+	t.is(slugify('bar'), 'bar');
+	t.is(slugify('bar 2'), 'bar-2');
+	t.is(slugify('bar 3'), 'bar-3');
+	t.is(slugify('bar'), 'bar-4');
+
+	slugify.reset();
+
+	// The collision check is case-insensitive, matching the counter itself.
+	t.is(slugify('baz'), 'baz');
+	t.is(slugify('BAZ 2', {lowercase: false}), 'BAZ-2');
+	t.is(slugify('baz'), 'baz-3');
+});
+
 test('preserve characters', t => {
 	t.is(slugify('foo#bar', {preserveCharacters: []}), 'foo-bar');
 	t.is(slugify('foo.bar', {preserveCharacters: []}), 'foo-bar');