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

basePath: urls in Router events include basePath #14845

Closed
Janpot opened this issue Jul 4, 2020 · 2 comments · Fixed by #14848
Closed

basePath: urls in Router events include basePath #14845

Janpot opened this issue Jul 4, 2020 · 2 comments · Fixed by #14848
Assignees
Milestone

Comments

@Janpot
Copy link
Contributor

Janpot commented Jul 4, 2020

Bug report

Describe the bug

Been picking apart the router logic a bit while working on #14827 and noticed that urls that are emitted in router events include the basepath.

Additionally, searching the test folder, there isn't a single test containing "routeChangeStart". It seems like these events are not covered by any tests.

To Reproduce

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

  1. configure a basePath
  2. add an pages/_app.js like:
    import { useEffect } from 'react'
    import { useRouter } from 'next/router'
    
    export default function MyApp({ Component, pageProps }) {
      const router = useRouter()
    
      useEffect(() => {
        const handleRouteChange = (url) => console.log('App is changing to: ', url)
        router.events.on('routeChangeStart', handleRouteChange)
        return () => router.events.off('routeChangeStart', handleRouteChange)
      }, [])
    
      return <Component {...pageProps} />
    }
  3. navigate links in your app and observe the console

Expected behavior

urls don't contain the basepath

@timneutkens
Copy link
Member

urls don't contain the basepath

I think this is needed because the events are generally used to track navigations in analytics services like Google Analytics.

@Timer Timer added this to the iteration 4 milestone Jul 5, 2020
@Timer Timer modified the milestones: iteration 4, iteration 5 Jul 13, 2020
@Timer Timer added the point: 2 label Jul 20, 2020
@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