From 502c4e3d58b30d6ac65c98fe881156ab677f7f66 Mon Sep 17 00:00:00 2001 From: asmyshlyaev177 Date: Thu, 1 Aug 2024 21:47:11 +0400 Subject: [PATCH] fix(useurlstate): ignore/preserve sp not defined in stateShape --- .../urlstate/next/useUrlState/useUrlState.ts | 7 +- .../useUrlStateBase/useUrlStateBase.test.ts | 80 +++++++++++++++- .../useUrlStateBase/useUrlStateBase.ts | 48 ++++++++-- tests/useUrlState/updateUrl.spec.ts | 93 ++++++++++++------- 4 files changed, 186 insertions(+), 42 deletions(-) diff --git a/packages/urlstate/next/useUrlState/useUrlState.ts b/packages/urlstate/next/useUrlState/useUrlState.ts index 00f4a3c..390cc6f 100644 --- a/packages/urlstate/next/useUrlState/useUrlState.ts +++ b/packages/urlstate/next/useUrlState/useUrlState.ts @@ -49,11 +49,12 @@ export function useUrlState( ); const sp = useSearchParams(); - React.useEffect(() => { - updateState( - parseSsrQs(Object.fromEntries([...sp.entries()]), defaultState), + const shapeKeys = Object.keys(defaultState); + const _sp = Object.fromEntries( + [...sp.entries()].filter(([key]) => shapeKeys.includes(key)), ); + updateState(parseSsrQs(_sp, defaultState)); }, [sp]); return { diff --git a/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts b/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts index abf296f..518d868 100644 --- a/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts +++ b/packages/urlstate/useUrlStateBase/useUrlStateBase.test.ts @@ -2,10 +2,9 @@ import { act, fireEvent, renderHook } from '@testing-library/react'; import { useUrlStateBase } from '.'; import * as sharedState from '../useSharedState'; +import * as urlEncode from '../useUrlEncode'; import * as utils from '../utils'; -// TODO: playwright tests with 2 components on 2 pages - type State = { str: string; num: number; @@ -105,6 +104,50 @@ describe('useUrlStateBase', () => { const fnArg = sharedStateSpy.mock.calls.slice(-1)[0][1]; expect(fnArg?.()).toStrictEqual(stateShape); }); + + describe('with existing queryParams', () => { + describe('state contains only fields from stateShape', () => { + it('ssr', () => { + jest.spyOn(utils, 'isSSR').mockReturnValue(true); + const sharedStateSpy = jest.spyOn(sharedState, 'useSharedState'); + const { result } = renderHook(() => + useUrlStateBase(stateShape, router, { key: 'value123' }), + ); + + expect(result.current.state).toStrictEqual(stateShape); + + expect(sharedStateSpy).toHaveBeenCalledTimes(1); + expect(sharedStateSpy).toHaveBeenNthCalledWith( + 1, + stateShape, + expect.any(Function), + ); + }); + + it('client', () => { + jest.spyOn(utils, 'isSSR').mockReturnValue(false); + const search = '?key=value123'; + const originalLocation = window.location; + jest.spyOn(window, 'location', 'get').mockImplementation(() => ({ + ...originalLocation, + search, + })); + const sharedStateSpy = jest.spyOn(sharedState, 'useSharedState'); + const { result } = renderHook(() => + useUrlStateBase(stateShape, router), + ); + + expect(result.current.state).toStrictEqual(stateShape); + + expect(sharedStateSpy).toHaveBeenCalledTimes(1); + expect(sharedStateSpy).toHaveBeenNthCalledWith( + 1, + stateShape, + expect.any(Function), + ); + }); + }); + }); }); describe('return state', () => { @@ -232,6 +275,39 @@ describe('useUrlStateBase', () => { someOpt: 123, }); }); + + describe('with existing queryParams', () => { + it('should only update fields from stateShape', () => { + jest.spyOn(utils, 'isSSR').mockReturnValue(false); + const sp = 'key=value123'; + const search = `?${sp}`; + const originalLocation = window.location; + jest.spyOn(window, 'location', 'get').mockImplementation(() => ({ + ...originalLocation, + search, + })); + const stringify = jest.fn().mockReturnValue(''); + jest.spyOn(urlEncode, 'useUrlEncode').mockImplementation( + jest.fn().mockReturnValue({ + parse: () => stateShape, + stringify, + }), + ); + const { result } = renderHook(() => + useUrlStateBase(stateShape, router), + ); + + const newState = { ...stateShape, num: 30 }; + act(() => { + result.current.updateUrl(newState); + }); + + expect(stringify).toHaveBeenCalledTimes(1); + const call = stringify.mock.calls[0]; + expect(call[0]).toStrictEqual(newState); + expect(call[1].toString()).toStrictEqual(sp); + }); + }); }); describe('updateState', () => { diff --git a/packages/urlstate/useUrlStateBase/useUrlStateBase.ts b/packages/urlstate/useUrlStateBase/useUrlStateBase.ts index 385646e..a843c85 100644 --- a/packages/urlstate/useUrlStateBase/useUrlStateBase.ts +++ b/packages/urlstate/useUrlStateBase/useUrlStateBase.ts @@ -33,14 +33,13 @@ export function useUrlStateBase( searchParams?: object, ) { const { parse, stringify } = useUrlEncode(defaultState); + // TODO: pass this block from useUrlState ? const { state, getState, setState } = useSharedState(defaultState, () => isSSR() - ? parseSsrQs(searchParams, defaultState) - : parse(window.location.search), + ? parseSsrQs(filterSsrSP(defaultState, searchParams), defaultState) + : parse(filterClientSP(defaultState)), ); - // TODO: href ? - useInsertionEffect(() => { // for history navigation const popCb = () => { @@ -62,15 +61,16 @@ export function useUrlStateBase( ) => { const currUrl = `${window.location.pathname}${window.location.search}${window.location.hash}`; const isFunc = typeof value === 'function'; + const otherParams = getOtherParams(defaultState); let newVal: T | DeepReadonly; let qStr: string; if (isFunc) { newVal = value(getState()); - qStr = stringify(newVal); + qStr = stringify(newVal, otherParams); } else { newVal = (value ?? getState()) as T; - qStr = stringify(newVal); + qStr = stringify(newVal, otherParams); } setState(newVal); @@ -94,6 +94,42 @@ export function useUrlStateBase( }; } +// need to use only common fields between shape and params, ignore undeclared SP +function filterClientSP(shape: T) { + const params = new URLSearchParams(window.location.search); + const shapeKeys = Object.keys(shape); + + const shapeParams = new URLSearchParams(); + [...params.entries()] + .filter(([key]) => shapeKeys.includes(key)) + .forEach(([key, value]) => shapeParams.set(key, value)); + + return shapeParams.toString(); +} + +function filterSsrSP(shape: T, searchParams?: object) { + const shapeKeys = Object.keys(shape); + + const result = Object.fromEntries( + Object.entries(searchParams || {}).filter(([key]) => + shapeKeys.includes(key), + ), + ); + return result as T; +} + +function getOtherParams(shape: T) { + const shapeKeys = Object.keys(shape); + const search = window.location.search; + const allParams = new URLSearchParams(search); + const params = new URLSearchParams(); + + allParams.forEach( + (value, key) => !shapeKeys.includes(key) && params.set(key, value), + ); + return params; +} + const popstateEv = 'popstate'; // eslint-disable-next-line @typescript-eslint/ban-types diff --git a/tests/useUrlState/updateUrl.spec.ts b/tests/useUrlState/updateUrl.spec.ts index 22479f3..a670978 100644 --- a/tests/useUrlState/updateUrl.spec.ts +++ b/tests/useUrlState/updateUrl.spec.ts @@ -1,8 +1,10 @@ import { expect, test } from '@playwright/test'; +import { toHaveUrl } from '../testUtils'; + const urls = ['/test-ssr', '/test-use-client', '/test-ssr-sp']; -test('sync', async ({ page, baseURL }) => { +test('sync', async ({ page }) => { for (const url of urls) { await page.goto(url); await page.waitForSelector('button[name="Reload page"]'); @@ -30,24 +32,21 @@ test('sync', async ({ page, baseURL }) => { await page.waitForTimeout(500); } - await expect(page).toHaveURL(`${baseURL}${url}`, { - timeout: 1000, - }); + await toHaveUrl(page, url, true); + await expect(page.getByTestId('parsed')).toHaveText(expectedText); // sync url await page.getByTestId('sync-empty').click(); - await page.waitForFunction(() => window.location.href.includes('?name=')); const expectedUrl = `?name=%E2%97%96My%2520Name&tags=%5B%7B%27id%27%3A%27%E2%97%961%27%2C%27value%27%3A%7B%27text%27%3A%27%E2%97%96React.js%27%2C%27time%27%3A%27%E2%97%962024-07-17T04%253A53%253A17.000Z%27%7D%7D%5D`; - await expect(page).toHaveURL(`${baseURL}${url}${expectedUrl}`, { - timeout: 1000, - }); + await toHaveUrl(page, `${url}${expectedUrl}`, true); + await expect(page.getByTestId('parsed')).toHaveText(expectedText); } }); -test('reset', async ({ page, baseURL }) => { +test('reset', async ({ page }) => { for (const url of urls) { await page.goto(url); await page.waitForSelector('button[name="Reload page"]'); @@ -76,9 +75,8 @@ test('reset', async ({ page, baseURL }) => { // sync url await page.getByTestId('sync-default').click(); - await expect(page).toHaveURL(`${baseURL}${url}`, { - timeout: 1000, - }); + await toHaveUrl(page, `${url}`); + await expect(page.getByTestId('parsed')).toHaveText(`{ "name": "", "agree to terms": false, @@ -87,16 +85,19 @@ test('reset', async ({ page, baseURL }) => { } }); -test('update', async ({ page, baseURL }) => { - for (const url of urls) { - await page.goto(url); - await page.waitForSelector('button[name="Reload page"]'); +test.describe('update', () => { + test('should work', async ({ page }) => { + for (const url of urls) { + await page.goto(url); + await page.waitForSelector('button[name="Reload page"]'); - await page.getByLabel('name').focus(); - await page.getByLabel('name').pressSequentially('My Name', { delay: 150 }); - await page.getByText('React.js').click(); + await page.getByLabel('name').focus(); + await page + .getByLabel('name') + .pressSequentially('My Name', { delay: 150 }); + await page.getByText('React.js').click(); - const expectedText = `{ + const expectedText = `{ "name": "My Name", "agree to terms": false, "tags": [ @@ -110,18 +111,17 @@ test('update', async ({ page, baseURL }) => { ] }`; - // syncing state but not url - await expect(page.getByTestId('parsed')).toHaveText(expectedText); + // syncing state but not url + await expect(page.getByTestId('parsed')).toHaveText(expectedText); - // update url - await page.getByTestId('sync-object').click(); + // update url + await page.getByTestId('sync-object').click(); - const expectedUrl = - '?name=%E2%97%96My%2520Name&age=%E2%88%9355&tags=%5B%7B%27id%27%3A%27%E2%97%961%27%2C%27value%27%3A%7B%27text%27%3A%27%E2%97%96React.js%27%2C%27time%27%3A%27%E2%97%962024-07-17T04%253A53%253A17.000Z%27%7D%7D%5D'; - await expect(page).toHaveURL(`${baseURL}${url}${expectedUrl}`, { - timeout: 1000, - }); - await expect(page.getByTestId('parsed')).toHaveText(`{ + const expectedUrl = + '?name=%E2%97%96My%2520Name&age=%E2%88%9355&tags=%5B%7B%27id%27%3A%27%E2%97%961%27%2C%27value%27%3A%7B%27text%27%3A%27%E2%97%96React.js%27%2C%27time%27%3A%27%E2%97%962024-07-17T04%253A53%253A17.000Z%27%7D%7D%5D'; + await toHaveUrl(page, `${url}${expectedUrl}`); + + await expect(page.getByTestId('parsed')).toHaveText(`{ "name": "My Name", "age": 55, "agree to terms": false, @@ -135,5 +135,36 @@ test('update', async ({ page, baseURL }) => { } ] }`); - } + } + }); + + test('should preserve existing query params', async ({ page }) => { + for (const url of urls) { + const sp = 'key1=someValue'; + await page.goto(`${url}?${sp}`); + await page.waitForSelector('button[name="Reload page"]'); + + await page.getByLabel('name').focus(); + await page + .getByLabel('name') + .pressSequentially('My Name', { delay: 150 }); + + const expectedText = `{ + "name": "My Name", + "agree to terms": false, + "tags": [] +}`; + + // syncing state but not url + await expect(page.getByTestId('parsed')).toHaveText(expectedText); + + // update url + await page.getByTestId('sync-empty').click(); + + const expectedUrl = `?${sp}&name=%E2%97%96My%2520Name`; + await toHaveUrl(page, `${url}${expectedUrl}`); + + await expect(page.getByTestId('parsed')).toHaveText(expectedText); + } + }); });