From 89f95210ace6d8e2e64b2113ecdcb154dbd29f96 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Wed, 23 Sep 2026 14:58:58 -0400 Subject: [PATCH] refactor: extract trailing-slash helpers into url utils The same trailing-slash rule was written inline in three places, so the three could drift apart as each gained its own edge-case handling. A shared pair of helpers keeps them together. The helpers absorb an absent value, since every caller reads an optional config field and would otherwise repeat the same guard. A wrong type still throws rather than passing through silently. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014V7YC1kW6f5ZFHtMNGcLKs --- src/utils/ajax.js | 3 ++- src/utils/api-fetch.js | 5 +++-- src/utils/url.js | 31 ++++++++++++++++++++++++++ src/utils/url.test.js | 49 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 src/utils/url.js create mode 100644 src/utils/url.test.js diff --git a/src/utils/ajax.js b/src/utils/ajax.js index 22c669113..1084a669b 100644 --- a/src/utils/ajax.js +++ b/src/utils/ajax.js @@ -3,6 +3,7 @@ */ import { getGBKit } from './bridge'; import { warn, debug } from './logger'; +import { stripTrailingSlash } from './url'; /** * Configure AJAX for use without authentication cookies. @@ -21,7 +22,7 @@ export function configureAjax() { window.wp.ajax.settings = window.wp.ajax.settings || {}; const { siteURL: rawSiteURL, authHeader } = getGBKit(); - const siteURL = rawSiteURL?.replace( /\/+$/, '' ); + const siteURL = stripTrailingSlash( rawSiteURL ); configureAjaxUrl( siteURL ); configureAjaxAuth( siteURL, authHeader ); configureMediaAjax(); diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 713059b69..d2ae7b683 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -10,6 +10,7 @@ import { __ } from '@wordpress/i18n'; */ import { getGBKit, POST_FALLBACKS } from './bridge'; import { info, error as logError } from './logger'; +import { ensureTrailingSlash, stripTrailingSlash } from './url'; /** * @typedef {import('@wordpress/api-fetch').APIFetchMiddleware} APIFetchMiddleware @@ -85,7 +86,7 @@ function apiPathModifierMiddleware( options, next ) { // single trailing slash. options.path = options.path.replace( /^(?\/?(?:[\w.-]+\/){2})/, - `$${ siteApiNamespace[ 0 ].replace( /\/+$/, '' ) }/` + `$${ ensureTrailingSlash( siteApiNamespace[ 0 ] ) }` ); } @@ -528,7 +529,7 @@ function siteIndexMiddleware( options, next ) { return next( options ); } - const home = siteURL?.replace( /\/+$/, '' ); + const home = stripTrailingSlash( siteURL ); return Promise.resolve( home ? { home } : {} ); } diff --git a/src/utils/url.js b/src/utils/url.js new file mode 100644 index 000000000..5a04b58c3 --- /dev/null +++ b/src/utils/url.js @@ -0,0 +1,31 @@ +/** + * Removes any trailing slashes from a URL or path. + * + * An absent value normalizes to an empty string, so an unset config field can + * be handed over directly. Any other non-string still throws, so a wrong type + * is not quietly passed through. + * + * @param {string} [value] The URL or path to normalize. + * @return {string} The value without a trailing slash. + */ +export function stripTrailingSlash( value ) { + return ( value ?? '' ).replace( /\/+$/, '' ); +} + +/** + * Normalizes a URL or path to exactly one trailing slash. + * + * An absent value stays absent rather than becoming a lone slash, which would + * resolve requests against the page root. A value that is already just `/` is + * kept, since that is a meaningful root. + * + * @param {string} [value] The URL or path to normalize. + * @return {string} The value with a single trailing slash, or an empty string. + */ +export function ensureTrailingSlash( value ) { + if ( value === undefined || value === null || value === '' ) { + return ''; + } + + return `${ stripTrailingSlash( value ) }/`; +} diff --git a/src/utils/url.test.js b/src/utils/url.test.js new file mode 100644 index 000000000..866fb95db --- /dev/null +++ b/src/utils/url.test.js @@ -0,0 +1,49 @@ +/** + * External dependencies + */ +import { describe, it, expect } from 'vitest'; + +/** + * Internal dependencies + */ +import { ensureTrailingSlash, stripTrailingSlash } from './url'; + +describe( 'stripTrailingSlash', () => { + it.each( [ + [ 'https://example.com/wp-json/', 'https://example.com/wp-json' ], + [ 'https://example.com/wp-json//', 'https://example.com/wp-json' ], + [ 'https://example.com/wp-json', 'https://example.com/wp-json' ], + [ 'sites/123/', 'sites/123' ], + [ '/', '' ], + [ '', '' ], + [ undefined, '' ], + [ null, '' ], + ] )( 'normalizes %s to %s', ( value, expected ) => { + expect( stripTrailingSlash( value ) ).toBe( expected ); + } ); + + // An absent value is expected; a wrong type is a caller bug worth surfacing. + it( 'throws on a non-string', () => { + expect( () => stripTrailingSlash( 42 ) ).toThrow( TypeError ); + } ); +} ); + +describe( 'ensureTrailingSlash', () => { + it.each( [ + [ 'https://example.com/wp-json', 'https://example.com/wp-json/' ], + [ 'https://example.com/wp-json/', 'https://example.com/wp-json/' ], + [ 'https://example.com/wp-json//', 'https://example.com/wp-json/' ], + [ 'sites/123', 'sites/123/' ], + // A lone slash is a meaningful root, unlike an absent value. + [ '/', '/' ], + [ '', '' ], + [ undefined, '' ], + [ null, '' ], + ] )( 'normalizes %s to %s', ( value, expected ) => { + expect( ensureTrailingSlash( value ) ).toBe( expected ); + } ); + + it( 'throws on a non-string', () => { + expect( () => ensureTrailingSlash( 42 ) ).toThrow( TypeError ); + } ); +} );