Repository: sindresorhus/normalize-url
Pull request #122 (2020-09-29 17:56:20)

Fix `removeSingleSlash` option adding slashes
Turns out the option caused single slashes to be added to URLs that had none because the `URL` constructor defaults pathname to `/` both on parse and .toString(), so I had to prevent overwriting `urlString` to be able to check if the passed URL's pathname is `/`.

Also added an entry for the option to `index.test-d.ts`.

sindresorhus: > Turns out the option caused single slashes to be added to URLs that had none because the URL constructor defaults pathname to / both on parse and .toString(), so I had to prevent overwriting urlString to be able to check if the passed URL's pathname is /.

Can you add a regression test for that?
silverwind: Two of the added test cases already cover this, they will fail on master:

https://github.com/sindresorhus/normalize-url/pull/122/files#diff-1dd241c4cd3fd1dd89c570cee98b79ddR138
https://github.com/sindresorhus/normalize-url/pull/122/files#diff-1dd241c4cd3fd1dd89c570cee98b79ddR148

It's not strictly a regression, more like an improvement for when `removeSingleSlash` is set to `false`.

Patch:
diff --git a/index.js b/index.js
index 8d4f989..e370d25 100644
--- a/index.js
+++ b/index.js
@@ -175,9 +175,15 @@ const normalizeUrl = (urlString, options) => {
 		urlObj.pathname = urlObj.pathname.replace(/\/$/, '');
 	}
 
+	const oldUrlString = urlString;
+
 	// Take advantage of many of the Node `url` normalizations
 	urlString = urlObj.toString();
 
+	if (!options.removeSingleSlash && urlObj.pathname === '/' && !oldUrlString.endsWith('/') && urlObj.hash === '') {
+		urlString = urlString.replace(/\/$/, '');
+	}
+
 	// Remove ending `/` unless removeSingleSlash is false
 	if ((options.removeTrailingSlash || urlObj.pathname === '/') && urlObj.hash === '' && options.removeSingleSlash) {
 		urlString = urlString.replace(/\/$/, '');


Test patch:
diff --git a/index.test-d.ts b/index.test-d.ts
index 104541f..7f2e308 100644
--- a/index.test-d.ts
+++ b/index.test-d.ts
@@ -16,6 +16,7 @@ normalizeUrl('www.sindresorhus.com?foo=bar&ref=test_ref', {
 	removeQueryParameters: ['ref', /test/]
 });
 normalizeUrl('http://sindresorhus.com/', {removeTrailingSlash: false});
+normalizeUrl('http://sindresorhus.com/', {removeSingleSlash: false});
 normalizeUrl('www.sindresorhus.com/foo/default.php', {
 	removeDirectoryIndex: [/^default\.[a-z]+$/, 'foo']
 });
diff --git a/test.js b/test.js
index 938e71b..df1b392 100644
--- a/test.js
+++ b/test.js
@@ -119,10 +119,15 @@ test('forceHttps option', t => {
 
 test('removeTrailingSlash option', t => {
 	const options = {removeTrailingSlash: false};
+	t.is(normalizeUrl('http://sindresorhus.com'), 'http://sindresorhus.com');
 	t.is(normalizeUrl('http://sindresorhus.com/'), 'http://sindresorhus.com');
+	t.is(normalizeUrl('http://sindresorhus.com', options), 'http://sindresorhus.com');
 	t.is(normalizeUrl('http://sindresorhus.com/', options), 'http://sindresorhus.com');
+	t.is(normalizeUrl('http://sindresorhus.com/redirect'), 'http://sindresorhus.com/redirect');
 	t.is(normalizeUrl('http://sindresorhus.com/redirect/'), 'http://sindresorhus.com/redirect');
 	t.is(normalizeUrl('http://sindresorhus.com/redirect/', options), 'http://sindresorhus.com/redirect/');
+	t.is(normalizeUrl('http://sindresorhus.com/redirect/', options), 'http://sindresorhus.com/redirect/');
+	t.is(normalizeUrl('http://sindresorhus.com/#/'), 'http://sindresorhus.com/#/');
 	t.is(normalizeUrl('http://sindresorhus.com/#/', options), 'http://sindresorhus.com/#/');
 	t.is(normalizeUrl('http://sindresorhus.com/?unicorns=true'), 'http://sindresorhus.com/?unicorns=true');
 	t.is(normalizeUrl('http://sindresorhus.com/?unicorns=true', options), 'http://sindresorhus.com/?unicorns=true');
@@ -130,7 +135,9 @@ test('removeTrailingSlash option', t => {
 
 test('removeSingleSlash option', t => {
 	const options = {removeSingleSlash: false};
+	t.is(normalizeUrl('https://sindresorhus.com', options), 'https://sindresorhus.com');
 	t.is(normalizeUrl('https://sindresorhus.com/', options), 'https://sindresorhus.com/');
+	t.is(normalizeUrl('https://sindresorhus.com/redirect', options), 'https://sindresorhus.com/redirect');
 	t.is(normalizeUrl('https://sindresorhus.com/redirect/', options), 'https://sindresorhus.com/redirect');
 	t.is(normalizeUrl('https://sindresorhus.com/#/', options), 'https://sindresorhus.com/#/');
 	t.is(normalizeUrl('https://sindresorhus.com/?unicorns=true', options), 'https://sindresorhus.com/?unicorns=true');
@@ -138,7 +145,9 @@ test('removeSingleSlash option', t => {
 
 test('removeSingleSlash option combined with removeTrailingSlash option', t => {
 	const options = {removeTrailingSlash: false, removeSingleSlash: false};
+	t.is(normalizeUrl('https://sindresorhus.com', options), 'https://sindresorhus.com');
 	t.is(normalizeUrl('https://sindresorhus.com/', options), 'https://sindresorhus.com/');
+	t.is(normalizeUrl('https://sindresorhus.com/redirect', options), 'https://sindresorhus.com/redirect');
 	t.is(normalizeUrl('https://sindresorhus.com/redirect/', options), 'https://sindresorhus.com/redirect/');
 	t.is(normalizeUrl('https://sindresorhus.com/#/', options), 'https://sindresorhus.com/#/');
 	t.is(normalizeUrl('https://sindresorhus.com/?unicorns=true', options), 'https://sindresorhus.com/?unicorns=true');