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.

Sign up for GitHub

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

Jump to bottom

Breaking type change on useRouter() in v13.0.2-canary.0 #42409

Closed
1 task done
jakejarvis opened this issue Nov 3, 2022 · 2 comments · Fixed by #42430
Closed
1 task done

Breaking type change on useRouter() in v13.0.2-canary.0 #42409

jakejarvis opened this issue Nov 3, 2022 · 2 comments · Fixed by #42430
Labels
bug Issue was opened via the bug report template.

Comments

@jakejarvis
Copy link
Contributor

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:30 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8103
Binaries:
  Node: 16.18.0
  npm: 8.19.2
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 13.0.2-canary.2
  eslint-config-next: 13.0.2-canary.2
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

A breaking change seems to be introduced in #41767 now that useRouter returns NextRouter | null instead of NextRouter by default. Calling router.pathname or router.events, for instance, throws a Type error: Object is possibly 'null' error.

Switching instances of useRouter() to useRouter(true) was an easy enough fix, but I expect lots more people will be hitting this speed bump when this change is released.

Expected Behavior

Is there a smart way to make this a non-breaking change? If this behavior is expected to be breaking, feel free to close this (but I'd still suggest emphasizing this change in the release notes to avoid a flood of bug reports! 😉)

Link to reproduction

vercel/examples#443 (review)

To Reproduce

Update from 13.0.1 to 13.0.2-canary.0 and up.

To fix with minimal changes, replace instances of useRouter() with useRouter(true).

Related: vercel/examples#443
@jakejarvis jakejarvis added the bug Issue was opened via the bug report template. label Nov 3, 2022
@11koukou
Copy link
Contributor

11koukou commented Nov 3, 2022

I confirm that

@ijjk ijjk mentioned this issue Nov 3, 2022
Merged
11 tasks
ijjk added a commit that referenced this issue Nov 3, 2022
@ijjk
Avoid breaking useRouter() type change (#42430)
152f51c 
Since `useRouter()` can still be used in `pages` alone and should always
be a valid instance we shouldn't change the default return type to avoid
a breaking type change.

Fixes: #42409

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid breaking useRouter() type change ijjk/next.js
2 participants
@jakejarvis @11koukou