diff --git a/.github/actions/cache-nextjs/action.yml b/.github/actions/cache-nextjs/action.yml index 6d3a2a3e8171..1612663b45fa 100644 --- a/.github/actions/cache-nextjs/action.yml +++ b/.github/actions/cache-nextjs/action.yml @@ -9,7 +9,7 @@ runs: using: 'composite' steps: - name: Cache .next/cache - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: ${{ github.workspace }}/.next/cache # Packages and source files both invalidate the cache. diff --git a/.github/actions/get-docs-early-access/action.yml b/.github/actions/get-docs-early-access/action.yml index 671694fa9eb1..760f4e4a126d 100644 --- a/.github/actions/get-docs-early-access/action.yml +++ b/.github/actions/get-docs-early-access/action.yml @@ -19,7 +19,7 @@ runs: run: npm run what-docs-early-access-branch - name: Clone - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: github/docs-early-access token: ${{ inputs.token }} diff --git a/.github/actions/node-npm-setup/action.yml b/.github/actions/node-npm-setup/action.yml index 8e4bb2999881..e25ce75a1f94 100644 --- a/.github/actions/node-npm-setup/action.yml +++ b/.github/actions/node-npm-setup/action.yml @@ -6,7 +6,7 @@ runs: using: 'composite' steps: - name: Cache node_modules - uses: actions/cache@v4 + uses: actions/cache@v6 id: cache-node_modules env: # Cache restore failures are acceptable, so keep the segment timeout short. @@ -17,7 +17,7 @@ runs: # The npm download cache only helps npm ci, so skip its 153 MB restore when node_modules hits. - name: Setup Node.js - uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6.0.0 + uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 with: node-version-file: 'package.json' package-manager-cache: false diff --git a/.github/actions/slack-alert/action.yml b/.github/actions/slack-alert/action.yml index 1940f4731bf9..64aa7438aadc 100644 --- a/.github/actions/slack-alert/action.yml +++ b/.github/actions/slack-alert/action.yml @@ -64,7 +64,7 @@ runs: } >> "$GITHUB_OUTPUT" - name: Send Slack notification if workflow fails - uses: slackapi/slack-github-action@45a88b9581bfab2566dc881e2cd66d334e621e2c # v3.0.3 + uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: method: chat.postMessage token: ${{ inputs.slack_token }} diff --git a/.github/actions/warmup-remotejson-cache/action.yml b/.github/actions/warmup-remotejson-cache/action.yml index 2d9220e09852..930640f055d5 100644 --- a/.github/actions/warmup-remotejson-cache/action.yml +++ b/.github/actions/warmup-remotejson-cache/action.yml @@ -12,7 +12,7 @@ runs: steps: # Restore a stable key, add to the cache, then save by SHA so the cache can grow without bound. - name: Cache .remotejson-cache (restore) - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v6 with: path: .remotejson-cache key: remotejson-cache- @@ -27,7 +27,7 @@ runs: - name: Cache .remotejson-cache (save) if: ${{ inputs.restore-only == '' }} - uses: actions/cache/save@v4 + uses: actions/cache/save@v6 with: path: .remotejson-cache key: remotejson-cache-${{ github.sha }} diff --git a/Dockerfile b/Dockerfile index dacef476896a..e88bff7bfb3d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ # Update the base image digest from the gh-base-noble package page: # https://github.com/github/gh-base-image/pkgs/container/gh-base-image%2Fgh-base-noble -FROM ghcr.io/github/gh-base-image/gh-base-noble:20261016-181622-g7dfd3798b@sha256:d41c754579434b9b6479a3d3786a6a84ad7be50ce64626bbc07579ab4c36302b AS base +FROM ghcr.io/github/gh-base-image/gh-base-noble:20261020-225427-g5ec2175c5@sha256:931ce7d43f6f3dbe28113cb12f5fac2206a0b08dad395172f6d73ce4f95f77f3 AS base # Install curl for NodeSource setup. # Install git for early-access and translation clones. # Ubuntu's nodejs package lags the Node LTS release line. diff --git a/package-lock.json b/package-lock.json index 8370a7190340..e07ed2e634b5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8685,9 +8685,9 @@ "license": "W3C" }, "node_modules/follow-redirects": { - "version": "1.16.0", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.0.tgz", - "integrity": "sha512-y5rN/uOsadFT/JfYwhxRS5R7Qce+g3zG97+JrtFZlC9klX/W5hD7iiLzScI4nZqUS7DNUdhPgw4xI8W2LuXlUw==", + "version": "1.16.1", + "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.1.tgz", + "integrity": "sha512-FNvFGzoMLWmE6Yj9spb/zjd7yiNCHiAW9/Tg9CXrQ8wuu32HtlJOwWO11OJafl5FfY3DxTdQ0vj42zU1kvv5jg==", "dev": true, "funding": [ { diff --git a/src/audit-logs/components/GroupedEvents.tsx b/src/audit-logs/components/GroupedEvents.tsx index f594bed04352..838760c5763d 100644 --- a/src/audit-logs/components/GroupedEvents.tsx +++ b/src/audit-logs/components/GroupedEvents.tsx @@ -28,12 +28,16 @@ export default function GroupedEvents({ auditLogEvents, category, categoryNote } .filter((link) => link && link !== 'N/A') .map(normalizeReferenceLink) - // Titles can contain ', ', so a single link keeps its whole title. - const titles = !event.docs_reference_titles - ? links - : links.length === 1 - ? [event.docs_reference_titles] - : event.docs_reference_titles.split(', ') + // Older generated data joins titles with ', ', and titles can contain ', ', + // so a single link keeps its whole title. + const storedTitles = event.docs_reference_titles + const titles = Array.isArray(storedTitles) + ? storedTitles + : !storedTitles + ? links + : links.length === 1 + ? [storedTitles] + : storedTitles.split(', ') return links.map((link, index) => ( diff --git a/src/audit-logs/lib/index.ts b/src/audit-logs/lib/index.ts index 6d5c52ad6952..158145ccc64c 100644 --- a/src/audit-logs/lib/index.ts +++ b/src/audit-logs/lib/index.ts @@ -194,9 +194,9 @@ async function computeReferenceLinksToMarkdown( async function resolveReferenceLinksToTitles( docsReferenceLinks: string, context: TitleResolutionContext, -): Promise { +): Promise { if (!docsReferenceLinks || docsReferenceLinks === 'N/A') { - return '' + return [] } const links = docsReferenceLinks @@ -233,7 +233,7 @@ async function resolveReferenceLinksToTitles( } } - return titles.join(', ') + return titles } export function getAuditLogEvents(page: string, version: string): AuditLogEventT[] { diff --git a/src/audit-logs/tests/unit/deduplication.ts b/src/audit-logs/tests/unit/deduplication.ts index d3fa146d726d..02de7856d209 100644 --- a/src/audit-logs/tests/unit/deduplication.ts +++ b/src/audit-logs/tests/unit/deduplication.ts @@ -141,7 +141,9 @@ describe('audit log deduplication', () => { expect(reconstructed[i].action).toBe(original[i].action) expect(reconstructed[i].description).toBe(original[i].description) expect(reconstructed[i].docs_reference_links).toBe(original[i].docs_reference_links) - expect(reconstructed[i].docs_reference_titles).toBe(original[i].docs_reference_titles) + expect(reconstructed[i].docs_reference_titles).toEqual( + original[i].docs_reference_titles, + ) if (original[i].fields) { expect(reconstructed[i].fields).toEqual(original[i].fields) } diff --git a/src/audit-logs/tests/unit/filter-events.ts b/src/audit-logs/tests/unit/filter-events.ts index 77e5559ee753..a92038a42cf9 100644 --- a/src/audit-logs/tests/unit/filter-events.ts +++ b/src/audit-logs/tests/unit/filter-events.ts @@ -201,7 +201,7 @@ describe('audit log event filtering', () => { pipelineConfig: { sha: '', appendedDescriptions: {} }, titleContext, }) - expect(filteredEvents[0].docs_reference_titles).toBe('Bar page title, /foo/missing-page') + expect(filteredEvents[0].docs_reference_titles).toEqual(['Bar page title', '/foo/missing-page']) }) test('resolves Markdown reference links without a language prefix', async () => { @@ -318,4 +318,38 @@ describe('audit log event filtering', () => { 'repo.create', ) }) + + test('keeps reference titles that contain commas intact', async () => { + const mockPage = (title: string) => ({ renderProp: async () => title }) + const titleContext = { + pages: { + '/en/sponsors/about-sponsorships-fees-and-taxes': mockPage( + 'About sponsorships, fees, and taxes', + ), + '/en/billing/upgrading-a-sponsorship': mockPage('Upgrading a sponsorship'), + }, + redirects: {}, + } as unknown as TitleResolutionContext + const eventsToProcess: RawAuditLogEventT[] = [ + { + action: 'sponsors.sponsor_sponsorship_create', + _allowlists: ['user'], + description: 'A sponsorship was created.', + docs_reference_links: + '/sponsors/about-sponsorships-fees-and-taxes, /billing/upgrading-a-sponsorship', + ghes: {}, + }, + ] + + const filteredEvents = await filterByAllowlistValues({ + eventsToCheck: eventsToProcess, + allowListValues: 'user', + pipelineConfig: { sha: '', appendedDescriptions: {} }, + titleContext, + }) + expect(filteredEvents[0].docs_reference_titles).toEqual([ + 'About sponsorships, fees, and taxes', + 'Upgrading a sponsorship', + ]) + }) }) diff --git a/src/audit-logs/types.ts b/src/audit-logs/types.ts index 35cafad284c4..c305982902da 100644 --- a/src/audit-logs/types.ts +++ b/src/audit-logs/types.ts @@ -7,7 +7,7 @@ export type AuditLogEventT = { description: string fields?: string[] docs_reference_links?: string - docs_reference_titles?: string + docs_reference_titles?: string | string[] } export type RawAuditLogEventT = { @@ -33,7 +33,7 @@ export type DeduplicatedAuditLogEntry = { action: string description: string docs_reference_links?: string - docs_reference_titles?: string + docs_reference_titles?: string | string[] fieldsIndex?: number } diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index 20aa436592d3..53463cddeb36 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -3092,6 +3092,32 @@ test.describe('LandingArticleGridWithFilter component', () => { await expect(page.getByTestId('article-grid').getByTestId('article-card')).toHaveCount(4) }) + test('typing in the header search keeps the selected category', async ({ page }) => { + // The category must not be in the URL at load, so the header's query state never sees it. + await page.goto('/get-started/article-grid-discovery') + await turnOffExperimentsInPage(page) + const trigger = page.getByTestId('filter-header').getByRole('button') + const cards = page.getByTestId('article-grid').getByTestId('article-card') + await trigger.click() + const menu = page.getByRole('menu', { name: 'Category', exact: true }) + await menu.getByRole('menuitemradio', { name: 'Testing', exact: true }).click() + await expect(page).toHaveURL(/articles-category=Testing/) + await expect(cards).toHaveCount(1) + + await page.getByTestId('toggle-search').click() + const input = page.getByTestId('overlay-search-input') + await input.pressSequentially('a') + await expect(page).toHaveURL(/search-overlay-input=a/) + await expect(page).toHaveURL(/articles-category=Testing/) + await expect(cards).toHaveCount(1) + + await input.press('Escape') + await expect(input).toHaveCount(0) + await expect(page).toHaveURL(/articles-category=Testing/) + await expect(trigger).toContainText('Testing') + await expect(cards).toHaveCount(1) + }) + test('category menu aligns with its inline label on desktop', async ({ page }) => { await page.setViewportSize({ width: 1400, height: 900 }) await page.goto('/get-started/article-grid-discovery?articles-category=Testing') diff --git a/src/frame/components/context/ArticleContext.tsx b/src/frame/components/context/ArticleContext.tsx index 1b74c547d8c8..605e3ca36766 100644 --- a/src/frame/components/context/ArticleContext.tsx +++ b/src/frame/components/context/ArticleContext.tsx @@ -46,8 +46,8 @@ export const useArticleContext = (): ArticleContextT => { return context } -const PagePathToVaFlowMapping: Record = { - 'content/account-and-profile/setting-up-and-managing-your-github-profile/managing-contribution-settings-on-your-profile/why-are-my-contributions-not-showing-up-on-my-profile.md': +export const PagePathToVaFlowMapping: Record = { + 'content/account-and-profile/how-tos/contribution-settings/troubleshooting-missing-contributions.md': 'contribution_troubleshooting', 'content/authentication/securing-your-account-with-two-factor-authentication-2fa/recovering-your-account-if-you-lose-your-2fa-credentials.md': '2fa', diff --git a/src/frame/lib/find-page.ts b/src/frame/lib/find-page.ts index 6c6329019d7e..87c49a850e03 100644 --- a/src/frame/lib/find-page.ts +++ b/src/frame/lib/find-page.ts @@ -1,5 +1,5 @@ import { getLanguageCode } from '@/frame/lib/patterns' -import getRedirect from '@/redirects/lib/get-redirect' +import getRedirect, { splitPathByLanguage } from '@/redirects/lib/get-redirect' import type { Context, Page } from '@/types' export default function findPage( @@ -19,11 +19,18 @@ export default function findPage( const page = pages[href] || (redirectedHref ? pages[redirectedHref] : undefined) if (page) return page + // Missing translated pages and hrefs without a language prefix fall back to English. + const [, withoutLanguage] = splitPathByLanguage(href) + const englishHrefs = new Set([withoutLanguage === '/' ? '/en' : `/en${withoutLanguage}`]) + // Also strip any two-letter prefix, such as a language not enabled here. const languageMatch = href.match(getLanguageCode) - const currentLang = getLanguageCode.test(href) && languageMatch ? languageMatch[1] : 'en' + if (languageMatch) englishHrefs.add(`/en${href.slice(languageMatch[0].length)}`) - // Missing translated pages fall back to English. - const englishHref = href.replace(`/${currentLang}/`, '/en/') - const redirectedEnglishHref = getRedirect(englishHref, redirectsContext) - return pages[englishHref] || (redirectedEnglishHref ? pages[redirectedEnglishHref] : undefined) + for (const englishHref of englishHrefs) { + const redirectedEnglishHref = getRedirect(englishHref, redirectsContext) + const englishPage = + pages[englishHref] || (redirectedEnglishHref ? pages[redirectedEnglishHref] : undefined) + if (englishPage) return englishPage + } + return undefined } diff --git a/src/frame/tests/find-page.ts b/src/frame/tests/find-page.ts index 513abc22e81a..65b600dd63c3 100644 --- a/src/frame/tests/find-page.ts +++ b/src/frame/tests/find-page.ts @@ -37,4 +37,37 @@ describe('find page', () => { expect(redirectedPage).toBeDefined() expect(typeof redirectedPage?.title).toBe('string') }) + + test('finds a page by an href without a language prefix', async () => { + const page = await Page.init({ + relativePath: 'page-with-redirects.md', + basePath: path.join(__dirname, '../../../src/fixtures/fixtures'), + languageCode: 'en', + }) + const permalink = page?.permalinks[0] + if (!page || !permalink) { + throw new Error('Page or permalink not found') + } + const pageMap = { [permalink.href]: page } as unknown as Record + + // The first two letters of these paths are not a language code. + expect(findPage(`${permalink.hrefWithoutLanguage}#a-fragment`, pageMap, {})).toBe(page) + expect(findPage(permalink.hrefWithoutLanguage, pageMap, {})).toBe(page) + }) + + test('falls back to English for a language prefix that is not enabled', async () => { + const page = await Page.init({ + relativePath: 'page-with-redirects.md', + basePath: path.join(__dirname, '../../../src/fixtures/fixtures'), + languageCode: 'en', + }) + const permalink = page?.permalinks[0] + if (!page || !permalink) { + throw new Error('Page or permalink not found') + } + const pageMap = { [permalink.href]: page } as unknown as Record + + // Unit tests run English-only, so `ja` is not an enabled language here. + expect(findPage(`/ja${permalink.hrefWithoutLanguage}`, pageMap, {})).toBe(page) + }) }) diff --git a/src/frame/tests/va-flow-mapping.test.ts b/src/frame/tests/va-flow-mapping.test.ts new file mode 100644 index 000000000000..c97cd5767527 --- /dev/null +++ b/src/frame/tests/va-flow-mapping.test.ts @@ -0,0 +1,12 @@ +import fs from 'fs' + +import { describe, expect, test } from 'vitest' + +import { PagePathToVaFlowMapping } from '@/frame/components/context/ArticleContext' + +describe('PagePathToVaFlowMapping', () => { + // Keys are matched against page.fullPath, so a moved article silently loses its VA iframe. + test.each(Object.keys(PagePathToVaFlowMapping))('%s exists', (pagePath) => { + expect(fs.existsSync(pagePath)).toBe(true) + }) +}) diff --git a/src/languages/lib/correct-translation-content.ts b/src/languages/lib/correct-translation-content.ts index cba38696f8f1..fe5eb3208930 100644 --- a/src/languages/lib/correct-translation-content.ts +++ b/src/languages/lib/correct-translation-content.ts @@ -1980,6 +1980,15 @@ export function correctTranslatedContentStrings( content = content.replace(/\{ +%([^%]+?)% *\}/g, '{%$1%}') + content = content.replace( + // eslint-disable-next-line no-control-regex + /\{%(-?)\s*([^\x00-\x7F][^\x00-\x7F\s]*(?: [^\x00-\x7F]+){0,3})(?:\.\s*|\s+)((?:variables|reusables)\.)?([a-z][\w-]*\.[\w-]+(?:\.[\w-]+)*)\s*(-?)%\}/g, + (_m, open: string, label: string, kind: string | undefined, path: string, close: string) => { + const ns = kind ?? (/变量|変数|변수|переменн/.test(label) ? 'variables.' : 'reusables.') + return `{%${open} data ${ns}${path} ${close}%}` + }, + ) + // eslint-disable-next-line no-control-regex -- Runs after per-language fixes; non-ASCII after {% is invalid. content = content.replace(/\{% (?=[^\x00-\x7F])/g, '') diff --git a/src/languages/tests/correct-translation-content.ts b/src/languages/tests/correct-translation-content.ts index 3389dbc09c45..73e10b059209 100644 --- a/src/languages/tests/correct-translation-content.ts +++ b/src/languages/tests/correct-translation-content.ts @@ -3797,4 +3797,21 @@ Para más información, consulta "[AUTOTITLE](/path)". expect(fix('{% warning %}\nx\n{% endwarning %}', 'ru')).toBe('x') }) }) + + describe('translated data label before an untranslated dotted path', () => { + test.each([ + ['zh', '{% 数据重用.profile.access_org %}', '{% data reusables.profile.access_org %}'], + ['zh', '{%- 数据重用.profile.access_org -%}', '{%- data reusables.profile.access_org -%}'], + ['ja', '{% データ 使い回し.user.dev-settings %}', '{% data reusables.user.dev-settings %}'], + ['ko', '{% 데이터 재사용가능한.a.b-c %}', '{% data reusables.a.b-c %}'], + ['ru', '{% переменных данных.product.name %}', '{% data variables.product.name %}'], + ])('fixes %s: %s', (code, before, after) => { + expect(fix(`a ${before} b`, code)).toBe(`a ${after} b`) + }) + + test('leaves already-correct tags unchanged', () => { + const ok = '{% data reusables.profile.access_org %} {% data variables.product.name %}' + for (const code of ['zh', 'ja', 'ko', 'ru']) expect(fix(ok, code)).toBe(ok) + }) + }) }) diff --git a/src/search/components/helpers/execute-search-actions.ts b/src/search/components/helpers/execute-search-actions.ts index 7ac09c3d7da9..95cf0997f5f4 100644 --- a/src/search/components/helpers/execute-search-actions.ts +++ b/src/search/components/helpers/execute-search-actions.ts @@ -17,13 +17,16 @@ const OVERLAY_PARAMS = [ 'query', ] +// Landing grid params describe the source page's grid, so a destination landing must not reuse them. +const PAGE_LOCAL_PARAMS = ['articles-category', 'articles-filter', 'articles-page'] + // Carries tool pickers, platform switches, and feature flags to the destination URL. // Not built from `router.query`, which also holds dynamic route params like `restPage`. // A `restPage` search param outranks the real path, so navigation would stay on this article. // Reads `window`, so call it only from event handlers, never during render. export function inheritableSearchParams(): URLSearchParams { const params = new URLSearchParams(window.location.search) - for (const key of OVERLAY_PARAMS) { + for (const key of [...OVERLAY_PARAMS, ...PAGE_LOCAL_PARAMS]) { params.delete(key) } return params diff --git a/src/search/components/hooks/useMultiQueryParams.ts b/src/search/components/hooks/useMultiQueryParams.ts index 76bccd455405..99a5248245c8 100644 --- a/src/search/components/hooks/useMultiQueryParams.ts +++ b/src/search/components/hooks/useMultiQueryParams.ts @@ -6,20 +6,50 @@ export type QueryParams = { 'search-overlay-input': string 'search-overlay-ask-ai': string // "true" or "" debug: string + // Landing pages filter article lists with these keys. 'articles-category': string 'articles-filter': string 'articles-page': string } -const initialKeys: (keyof QueryParams)[] = [ - 'search-overlay-input', - 'search-overlay-ask-ai', - 'debug', - // Landing pages filter article lists with these keys. - 'articles-category', - 'articles-filter', - 'articles-page', -] +// Other params in `query` are left as they are because another hook instance may own them. +export function applyQueryParams( + query: string, + params: QueryParams, + keys: Iterable, +): URLSearchParams { + const searchParams = new URLSearchParams(query) + for (const key of keys) { + if (key === 'search-overlay-ask-ai') { + if (params[key] === 'true') { + searchParams.set(key, 'true') + } else { + searchParams.delete(key) + } + } else { + if (params[key]) { + searchParams.set(key, params[key]) + } else { + searchParams.delete(key) + } + } + } + return searchParams +} + +// `asPath` gives the route only, since its query can be stale when a debounced update runs. +export function buildParamsUrl( + asPath: string, + liveQuery: string, + locale: string | undefined, + params: QueryParams, + keys: Iterable, +): { url: string; as: string } { + const [asPathRoot] = asPath.split('#')[0].split('?') + const searchParams = applyQueryParams(liveQuery, params, keys) + const paramsString = searchParams.toString() ? `?${searchParams.toString()}` : '' + return buildShallowQueryUrls(asPathRoot, paramsString, locale) +} // Updating related query params in one state change prevents router races. export function useMultiQueryParams(options?: { @@ -49,6 +79,9 @@ export function useMultiQueryParams(options?: { } const [params, setParams] = useState(getInitialParams) + // Write only updated keys: each instance holds stale values for keys another instance owns. + // Excluded keys keep state across history navigation, so state owns them from the start. + const ownedKeysRef = useRef(new Set(excludeFromHistory)) // React state owns query params after the route path initializes them. useEffect(() => { @@ -77,33 +110,24 @@ export function useMultiQueryParams(options?: { const updateParams = useCallback( (updates: Partial, shouldPushHistory = false) => { + for (const key of Object.keys(updates) as (keyof QueryParams)[]) { + ownedKeysRef.current.add(key) + } // A functional update keeps params out of this callback's dependencies. setParams((currentParams) => { const newParams = { ...currentParams, ...updates } - const [asPathWithoutHash] = router.asPath.split('#') - const [asPathRoot, asPathQuery = ''] = asPathWithoutHash.split('?') - const searchParams = new URLSearchParams(asPathQuery) - for (const key of initialKeys) { - if (key === 'search-overlay-ask-ai') { - if (newParams[key] === 'true') { - searchParams.set(key, 'true') - } else { - searchParams.delete(key) - } - } else { - if (newParams[key]) { - searchParams.set(key, newParams[key]) - } else { - searchParams.delete(key) - } - } - } - const paramsString = searchParams.toString() ? `?${searchParams.toString()}` : '' - const { url, as } = buildShallowQueryUrls(asPathRoot, paramsString, router.locale) // Debounce the router push so we don't push a new URL for every keystroke if (pushTimeoutRef.current) clearTimeout(pushTimeoutRef.current) pushTimeoutRef.current = setTimeout(async () => { + // Read the live query here so updates another instance made during the debounce stay. + const { url, as } = buildParamsUrl( + router.asPath, + window.location.search, + router.locale, + newParams, + ownedKeysRef.current, + ) // Preserve scroll position so component scroll logic stays in control. const scrollY = window.scrollY const scrollX = window.scrollX diff --git a/src/search/tests/execute-search-actions.ts b/src/search/tests/execute-search-actions.ts index a5491ba36def..e462492f290a 100644 --- a/src/search/tests/execute-search-actions.ts +++ b/src/search/tests/execute-search-actions.ts @@ -1,9 +1,13 @@ import { afterEach, describe, expect, test, vi } from 'vitest' import type { NextRouter } from 'next/router' -import { executeGeneralSearch } from '@/search/components/helpers/execute-search-actions' +import { + executeGeneralSearch, + withInheritedParams, +} from '@/search/components/helpers/execute-search-actions' import { DEFAULT_VERSION } from '@/versions/components/useVersion' +// Importing the events module starts a repeating timer that flushes analytics. vi.mock('@/events/components/events', () => ({ sendEvent: vi.fn() })) function setup(search: string) { @@ -46,3 +50,12 @@ describe('executeGeneralSearch', () => { expect(url.searchParams.has('page')).toBe(false) }) }) + +describe('withInheritedParams', () => { + test('drops overlay and landing grid params but keeps the rest', () => { + const search = + '?tool=cli&search-overlay-open=true&search-overlay-input=a&articles-category=Agents&articles-page=2&articles-filter=foo' + vi.stubGlobal('window', { location: { search } }) + expect(withInheritedParams('/en/actions')).toBe('/en/actions?tool=cli') + }) +}) diff --git a/src/search/tests/use-multi-query-params.ts b/src/search/tests/use-multi-query-params.ts new file mode 100644 index 000000000000..7897c117e92a --- /dev/null +++ b/src/search/tests/use-multi-query-params.ts @@ -0,0 +1,79 @@ +import { describe, expect, test } from 'vitest' + +import { applyQueryParams, buildParamsUrl } from '@/search/components/hooks/useMultiQueryParams' +import type { QueryParams } from '@/search/components/hooks/useMultiQueryParams' + +const emptyParams: QueryParams = { + 'search-overlay-input': '', + 'search-overlay-ask-ai': '', + debug: '', + 'articles-category': '', + 'articles-filter': '', + 'articles-page': '', +} + +describe('applyQueryParams', () => { + test('search overlay updates keep the article grid params', () => { + const query = 'articles-category=Tutorials&articles-page=2&articles-filter=foo' + const params = { ...emptyParams, 'search-overlay-input': 'a' } + const result = applyQueryParams(`${query}&search-overlay-open=true`, params, [ + 'search-overlay-input', + 'search-overlay-ask-ai', + ]) + expect(result.toString()).toBe(`${query}&search-overlay-open=true&search-overlay-input=a`) + }) + + test('article grid updates keep the search overlay params', () => { + const params = { ...emptyParams, 'articles-category': 'Agents' } + const result = applyQueryParams( + 'search-overlay-open=true&search-overlay-input=a&search-overlay-ask-ai=true', + params, + ['articles-category', 'articles-page'], + ) + expect(result.toString()).toBe( + 'search-overlay-open=true&search-overlay-input=a&search-overlay-ask-ai=true&articles-category=Agents', + ) + }) + + test('empty values delete only the given keys', () => { + const result = applyQueryParams('articles-category=Agents&articles-page=2', emptyParams, [ + 'articles-page', + ]) + expect(result.toString()).toBe('articles-category=Agents') + }) + + test('writes search-overlay-ask-ai only when it is true', () => { + const keys: (keyof QueryParams)[] = ['search-overlay-ask-ai'] + const askAI = { ...emptyParams, 'search-overlay-ask-ai': 'true' } + expect(applyQueryParams('', askAI, keys).toString()).toBe('search-overlay-ask-ai=true') + const other = { ...emptyParams, 'search-overlay-ask-ai': 'yes' } + expect(applyQueryParams('search-overlay-ask-ai=true', other, keys).toString()).toBe('') + }) +}) + +describe('buildParamsUrl', () => { + test('a header update keeps a category written while it waited', () => { + // The header scheduled its update before the grid's category push landed in the URL. + const scheduledAsPath = '/copilot?search-overlay-open=true' + const liveQuery = '?search-overlay-open=true&articles-category=Testing' + const params = { ...emptyParams, 'search-overlay-input': 'a' } + const { url, as } = buildParamsUrl(scheduledAsPath, liveQuery, 'en', params, [ + 'search-overlay-input', + ]) + const expected = + '/en/copilot?search-overlay-open=true&articles-category=Testing&search-overlay-input=a' + expect(url).toBe(expected) + expect(as).toBe(expected) + }) + + test('drops the hash and keeps the home page locale only in `as`', () => { + const params = { ...emptyParams, 'articles-page': '2' } + expect(buildParamsUrl('/#top', '', 'en', params, ['articles-page'])).toEqual({ + url: '/?articles-page=2', + as: '/en?articles-page=2', + }) + expect( + buildParamsUrl('/copilot#top', '?articles-page=2', 'en', emptyParams, ['articles-page']), + ).toEqual({ url: '/en/copilot', as: '/en/copilot' }) + }) +})