Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router.push breaks on relative paths for old browsers (Chrome 49, IE11) #11702

Closed
acifani opened this issue Apr 6, 2020 · 4 comments · Fixed by #15209
Closed

Router.push breaks on relative paths for old browsers (Chrome 49, IE11) #11702

acifani opened this issue Apr 6, 2020 · 4 comments · Fixed by #15209
Assignees
Milestone

Comments

@acifani
Copy link
Contributor

acifani commented Apr 6, 2020

Bug report

Describe the bug

When using router.push with a relative url on older browsers, it will throw an error (in dev) or silently fail (in prod). E.g.

function Component() {
  const router = useRouter()
  return <button onClick={() => router.push('/foo')}>Click me</button>
}

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Trigger a Router.push call as explained above
  2. If URL is supported by the browser, all is well
  3. If URL is polyfilled you'll get a Invalid href passed to router

Expected behavior

There should be no discrepancies between the 2 behaviors

System information

  • OS: macOS, Windows
  • Browser: tested on Chrome 49, IE11, probably others
  • Version of Next.js: 9.3.4

Additional context

I tracked the issue down to this call here

https://github.com/zeit/next.js/blob/b274fe39d38111588e625342a89dc568ad38fff5/packages/next/next-server/lib/router/router.ts#L395

this will evaluate differently based on whether or not the polyfill is used.
When the native URL is used, protocol will be null, when the polyfill version is used, protocol will be http://, thus throwing the error just below the mentioned line.

This is caused by the polyfill not throwing when an invalid URL is created, like new URL('/foo'), and triggering a different behavior on native-url.
A PR has just been merged regarding this bug and will hopefully get published soon lifaon74/url-polyfill#54

@u840903
Copy link

u840903 commented Apr 23, 2020

Upgrading to 9.3.5 solved it for me.

@acifani
Copy link
Contributor Author

acifani commented Apr 24, 2020

@janjarfalk unfortunately, it did not fix it for me. Do you remember which error you were experiencing before the upgrade? What browsers did you test it with?

This was referenced May 12, 2020
@Timer Timer self-assigned this Jul 16, 2020
@kodiakhq kodiakhq bot closed this as completed in #15209 Jul 16, 2020
kodiakhq bot pushed a commit that referenced this issue Jul 16, 2020
This replaces the `url-polyfill` package with the `core-js` version which handles more edge cases in legacy browsers.

Closes #11702
Fixes #15194
@Timer Timer added this to the iteration 5 milestone Jul 16, 2020
@Timer
Copy link
Member

Timer commented Jul 16, 2020

This should be fixed in next@^9.4.5-canary.36!

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants