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

next/link legacyBehavior default value in tests incorrect as of v13 #42621

Closed
1 task done
ValentinH opened this issue Nov 8, 2022 · 2 comments · Fixed by #42623
Closed
1 task done

next/link legacyBehavior default value in tests incorrect as of v13 #42621

ValentinH opened this issue Nov 8, 2022 · 2 comments · Fixed by #42623
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@ValentinH
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 21.6.0: Mon Aug 22 20:19:52 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T6000
Binaries:
Node: 16.13.2
npm: 8.1.2
Yarn: 3.1.1
pnpm: N/A
Relevant packages:
next: 13.0.2
eslint-config-next: 12.3.1
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

After upgrading to v13 and replacing all our next/link to use the new syntax, we started to have some tests failing due to an error saying React.Children.only expected to receive a single React element child.

We are indeed passing multiple children to <Link> but it works fine in our app, just not in tests. After digging a bit, the error is thrown at this line. I noticed that this line is inside a if (legacyBehavior) condition which doesn't make sense in our case as we are not using this prop.

I therefore noticed that the default value of legacyBehavior is Boolean(process.env.__NEXT_NEW_LINK_BEHAVIOR) !== true (here). After logging this env value, I saw it was undefined which explains why legacyBehavior ends up being true.

Expected Behavior

As the new link behaviour is the default as of v13, I think this condition should be at least reversed or even removed so that the default behaviour is not the legacy one if __NEXT_NEW_LINK_BEHAVIOR is not set.

Link to reproduction

https://github.com/ValentinH/next13-link-test-issue/blob/main/src/example.test.js

To Reproduce

Clone https://github.com/ValentinH/next13-link-test-issue and run yarn test.

@ValentinH ValentinH added the bug Issue was opened via the bug report template. label Nov 8, 2022
@ValentinH
Copy link
Contributor Author

For the moment, I fixed this issue by adding __NEXT_NEW_LINK_BEHAVIOR=true to my .env.test file.

@balazsorban44 balazsorban44 added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Nov 9, 2022
ijjk added a commit that referenced this issue Dec 1, 2022
## Bug
Fixes #42621

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

Co-authored-by: JJ Kasper <jj@jjsweb.site>
@github-actions
Copy link
Contributor

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 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants