Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@ import isDynamicRouteSuffix from './isDynamicRouteSuffix';
import splitPathAndQuery from './splitPathAndQuery';

/**
* Merges two query strings into one. If both contain the same key,
* the error is thrown.
* Merges two query strings into one. When both contain the same key, the suffix
* value wins (it is the intended destination of the navigation) and the duplicate
* from the base path is overwritten. A differing value is logged but never fatal,
* because throwing here crashes the app on a legitimate navigation.
* @param baseQuery - The query string of the base path
* @param suffixQuery - The query string of the suffix
* @returns The merged query string or an empty string if both are empty
*
* @private - Internal helper. Do not export or use outside this file.
*
* @example
* mergeQueryStrings('foo=bar', 'foo=baz') => '?foo=bar&baz=qux'
* mergeQueryStrings('foo=bar', 'foo=baz') => throws an error
* mergeQueryStrings('foo=bar', 'baz=qux') => '?foo=bar&baz=qux'
* mergeQueryStrings('action=edit', 'action=create') => '?action=create' (suffix wins)
*/
const mergeQueryStrings = (baseQuery = '', suffixQuery = ''): string => {
if (!baseQuery && !suffixQuery) {
Expand All @@ -27,8 +29,12 @@ const mergeQueryStrings = (baseQuery = '', suffixQuery = ''): string => {
const suffixParams = new URLSearchParams(suffixQuery);
const suffixParamsEntries = suffixParams.entries();
for (const [key, value] of suffixParamsEntries) {
if (params.has(key)) {
throw new Error(`[createDynamicRoute] Query param "${key}" exists in both base path and dynamic suffix. This is not allowed.`);
if (params.has(key) && params.get(key) !== value) {
Log.warn('[createDynamicRoute.ts] Query param exists in both base path and dynamic suffix with different values; suffix value takes precedence', {
key,
baseValue: params.get(key),
suffixValue: value,
});
}
params.set(key, value);
}
Expand Down
24 changes: 22 additions & 2 deletions tests/navigation/createDynamicRouteTests.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Log from '@libs/Log';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation from '@libs/Navigation/Navigation';

Expand Down Expand Up @@ -25,6 +26,8 @@ jest.mock('@src/ROUTES', () => ({

describe('createDynamicRoute', () => {
const mockGetActiveRoute = jest.mocked(Navigation.getActiveRoute);
// eslint-disable-next-line @typescript-eslint/unbound-method -- jest.fn() mock doesn't rely on `this` binding
const mockLogWarn = jest.mocked(Log.warn);

beforeEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -141,13 +144,30 @@ describe('createDynamicRoute', () => {
expect(result).toBe(expectedPath);
});

it('should throw an error when suffix query param collides with base path query param', () => {
it('should let the suffix query param win when it collides with a base path query param', () => {
const activeRoute = 'settings/profile/address?country=GB';
const suffixWithQuery = 'country?country=US';
const expectedPath = 'settings/profile/address/country?country=US';

mockGetActiveRoute.mockReturnValue(activeRoute);

const result = createDynamicRoute(suffixWithQuery);

expect(result).toBe(expectedPath);
expect(mockLogWarn).toHaveBeenCalled();
});

it('should not warn when a colliding query param has the same value in base and suffix', () => {
const activeRoute = 'settings/profile/address?country=US';
const suffixWithQuery = 'country?country=US';
const expectedPath = 'settings/profile/address/country?country=US';

mockGetActiveRoute.mockReturnValue(activeRoute);

expect(() => createDynamicRoute(suffixWithQuery)).toThrow('[createDynamicRoute] Query param "country" exists in both base path and dynamic suffix. This is not allowed.');
const result = createDynamicRoute(suffixWithQuery);

expect(result).toBe(expectedPath);
expect(mockLogWarn).not.toHaveBeenCalled();
});

it('should append parametric suffix with single param to path', () => {
Expand Down
Loading