next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Error thrown when window.history.pushState is called with "" as the first argument

Open adamsullovey opened this issue 1 year ago • 6 comments

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/github/vercel/next.js/tree/canary/examples/reproduction-template (or https://codesandbox.io/p/devbox/confident-pateu-go8s7s)

To Reproduce

  1. open up the sample Next.js app @ https://codesandbox.io/p/sandbox/github/vercel/next.js/tree/canary/examples/reproduction-template (or https://codesandbox.io/p/devbox/confident-pateu-go8s7s)
  2. open the browser dev tools for the site preview
  3. run window.history.pushState("", null, "/help") in the browser console

Screenshot 2024-07-21 at 5 52 32 PM

Current vs. Expected behavior

Current behaviour

An error is thrown:

Uncaught TypeError: Cannot create property '__NA' on string ''
    at copyNextJsInternalHistoryState (app-router.js:125:19)
    at History.pushState (app-router.js:375:20)
    at <anonymous>:1:16

Expected behaviour

The URL in the navigation bar should change to include the path /help

Provide environment information

This is taken from a local project:

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 23.5.0: Wed May  1 20:09:52 PDT 2024; root:xnu-10063.121.3~5/RELEASE_X86_64
  Available memory (MB): 16384
  Available CPU cores: 16
Binaries:
  Node: 20.12.2
  npm: 10.5.0
  Yarn: 1.22.19
  pnpm: 8.14.1
Relevant Packages:
  next: 14.2.3 // There is a newer version (14.2.5) available, upgrade recommended! 
  eslint-config-next: 14.2.2
  react: 18.3.1
  react-dom: 18.3.1
  typescript: 5.5.3
Next.js Config:
  output: N/A
 ⚠ There is a newer version (14.2.5) available, upgrade recommended! 
   Please try the latest canary version (`npm install next@canary`) to confirm the issue still exists before creating a new issue.
   Read more - https://nextjs.org/docs/messages/opening-an-issue

The Code Sandbox link above has the same issue and uses Next.js `15.0.0-canary.68`

Which area(s) are affected? (Select all that apply)

Navigation

Which stage(s) are affected? (Select all that apply)

next dev (local), next start (local), Other (Deployed)

Additional context

I have received a not open-source 3rd party library to integrate with a Next.js site which updates URLs with calls like window.history.pushState("", null, "/some-path"). Passing "" as the first argument should be ok according MDN since "" is serializable. It does not trigger an error outside of Next.js. Unfortunately calling Next.js' version of pushState with a blank string as a first argument causes a crash and the 3rd party library stops working.

The crash originates here: https://github.com/vercel/next.js/blob/e0ed599f43d3a0114e19f7ba6522270e5384e190/packages/next/src/client/components/app-router.tsx#L170

  if (data == null) data = {}
  const currentState = window.history.state
  const __NA = currentState?.__NA
  if (__NA) {
    data.__NA = __NA
  }

If data is "", data will not be reassigned to {}. Later data.__NA = __NA will try to create a new property on "" which is not allowed in strict mode.

I'm happy to put up a PR that changes this if I can get some direction on what the right fix is. I don't know if changing to if (!data) data = {} or if (data == null || data == "") data = {} or something else would preserve more intended behaviours.

adamsullovey avatar Jul 21 '24 21:07 adamsullovey

You didn't provide the right link to that codesandbox, it is linking to the template starter.

I think the issue here is that Next.js stores its own state into the history, since The state object can be anything that can be serialized.:

{
    "__NA": true,
    "__PRIVATE_NEXTJS_INTERNALS_TREE": [
        "",
        {
            "children": [
                "__PAGE__",
                {}
            ]
        },
        null,
        null,
        true
    ]
}

That's also valid from their side... 🤔

While:

  if (data == null || typeof data === 'string') data = {}

And the link kind of checks, would most likely stop the issue, I wonder what'll happen to the rest of the app. We need a core maintainer up in here.

icyJoseph avatar Jul 22 '24 08:07 icyJoseph

I'm encountering the same issue. It's not only an issue with history.pushState, but also with history.replaceState.

In my case it's also a third party library that makes the call to replaceState and passes "" as first argument. So it's difficult to work around this issue.

history.replaceState('', document.title, loc.pathname);

The issue can easily be fixed by updating this line:

// Old version: 
if (data == null) data = {}

// Suggested version:
if (data === null || data === "") data = {}

BramMeerten avatar Oct 30 '24 08:10 BramMeerten

I've worked around the issue by adding this code (loaded after the NextJS code), you can do the same for pushState:

const originalFunction = window.history.replaceState.bind(window.history);
window.history.replaceState = function(data: any, unused: string, url?: string | URL | null) {
	const newData = data === "" ? null : data;
	return originalFunction(newData, unused, url);
}

BramMeerten avatar Oct 30 '24 08:10 BramMeerten

@BramMeerten thanks for your workaround - could you elaborate where you have applied this as I am having the same issue and am new to next.

daviddigital avatar Apr 14 '25 16:04 daviddigital

@daviddigital I've added it in my root layout.tsx file:

export default function RootLayout({children}: {children: React.ReactNode}) {
	return (
		<html ...>
			...
			<body>
				<AppContainer>{children}</AppContainer>
				<Script type="text/javascript" src="/hotfix.js" />
			</body>
		</html>
	);
}

And the workaround code is in public/hotfix.js.

BramMeerten avatar Apr 14 '25 16:04 BramMeerten

@BramMeerten Thanks so much this is solving an issue I had with Snipcart in Nextjs

notflip avatar Dec 03 '25 10:12 notflip