diff --git a/packages/react-router/src/Asset.tsx b/packages/react-router/src/Asset.tsx index 5e4f159f52..4660340fb4 100644 --- a/packages/react-router/src/Asset.tsx +++ b/packages/react-router/src/Asset.tsx @@ -159,6 +159,14 @@ function Script({ ) } + // `attrs` is rebuilt as a fresh object on every head-tag computation, so + // keying this effect off object identity removes and re-injects `src` + // scripts on every navigation that changes head tags - re-executing + // third-party scripts (analytics, tag managers, widgets) each time. + // Serialize to a stable key so the effect only re-runs when the script + // actually changes. + const attrsKey = JSON.stringify(attrs ?? null) + React.useEffect(() => { if (dataScript) return @@ -217,7 +225,8 @@ function Script({ } return undefined - }, [attrs, children, dataScript]) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [attrsKey, children, dataScript]) // --- Server rendering --- if (isServer ?? router.isServer) { diff --git a/packages/react-router/tests/head-script-src-navigation.test.tsx b/packages/react-router/tests/head-script-src-navigation.test.tsx new file mode 100644 index 0000000000..f801642dcd --- /dev/null +++ b/packages/react-router/tests/head-script-src-navigation.test.tsx @@ -0,0 +1,105 @@ +import { createPortal } from 'react-dom' +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' +import { afterEach, expect, test, vi } from 'vitest' +import { + HeadContent, + Link, + Outlet, + RouterProvider, + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, +} from '../src' + +afterEach(() => { + cleanup() + document.head.innerHTML = '' + vi.restoreAllMocks() +}) + +// A head() script with `src` must be injected (and therefore executed) exactly +// once. Before the fix, the injection effect keyed off the `attrs` object +// identity, which changes on every navigation that rebuilds head tags - so +// the script was removed and re-injected (re-executed) on every navigation. +test('head() scripts with src are not re-injected on client-side navigations', async () => { + const rootRoute = createRootRoute({ + head: () => ({ + scripts: [{ src: '/track-test.js' }], + }), + component: () => ( + <> + {createPortal(, document.head)} + index + + pool + + + + ), + }) + const indexRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/', + head: () => ({ meta: [{ title: 'Home Page' }] }), + component: () =>
index
, + }) + const poolRoute = createRoute({ + getParentRoute: () => rootRoute, + path: '/pool/$id', + head: () => ({ meta: [{ title: 'Pool Detail' }] }), + component: () =>
pool
, + }) + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, poolRoute]), + history: createMemoryHistory({ initialEntries: ['/'] }), + }) + + const injections: Array = [] + const removals: Array = [] + const originalAppendChild = document.head.appendChild.bind(document.head) + vi.spyOn(document.head, 'appendChild').mockImplementation((node: any) => { + if ( + node instanceof HTMLScriptElement && + node.getAttribute('src') === '/track-test.js' + ) { + injections.push(node) + } + return originalAppendChild(node) + }) + vi.spyOn(HTMLScriptElement.prototype, 'remove').mockImplementation( + function (this: HTMLScriptElement) { + if (this.getAttribute('src') === '/track-test.js') { + removals.push(this) + } + // Element.prototype.remove + this.parentNode?.removeChild(this) + }, + ) + + render() + expect(await screen.findByTestId('index-page')).toBeInTheDocument() + await waitFor(() => expect(document.title).toBe('Home Page')) + expect(injections).toHaveLength(1) + + fireEvent.click(screen.getByRole('link', { name: 'pool' })) + expect(await screen.findByTestId('pool-page')).toBeInTheDocument() + await waitFor(() => expect(document.title).toBe('Pool Detail')) + + fireEvent.click(screen.getByRole('link', { name: 'index' })) + expect(await screen.findByTestId('index-page')).toBeInTheDocument() + await waitFor(() => expect(document.title).toBe('Home Page')) + + // Every re-injection re-executes the script in a real browser. + expect(injections).toHaveLength(1) + expect(removals).toHaveLength(0) + expect( + document.head.querySelectorAll('script[src="/track-test.js"]'), + ).toHaveLength(1) +})