-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
NextJS: Support next/navigation in Next.js v13 #20065
Conversation
code/frameworks/nextjs/template/stories_default-js/Navigation.stories.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks awesome!
code/frameworks/nextjs/template/stories_default-js/Navigation.stories.jsx
Show resolved
Hide resolved
Co-authored-by: Kyle Gach <kyle.gach@gmail.com>
Co-authored-by: Kyle Gach <kyle.gach@gmail.com>
Co-authored-by: Kyle Gach <kyle.gach@gmail.com>
Co-authored-by: Michael Shilman <shilman@users.noreply.github.com>
a69be59
to
66f9945
Compare
…pport-next-navigation
5fa671a
to
544d8e5
Compare
Co-authored-by: Yann Braga <yannbf@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!!! Small changes & questions below
}); | ||
|
||
test.describe('next/navigation', () => { | ||
test.skip( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder what this will do to the CI status report? cc @kasperpeulen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess everything will fail. But @kasperpeulen is working on adding metadata to tests, so I hope there's a way to configure this, either by adding the skip descrition or something, which allows us to get the results right
Co-authored-by: Michael Shilman <shilman@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with Shilman's comments, but otherwise, this looks great to me! Nice work, Valentin!
Co-authored-by: Kyle Gach <kyle.gach@gmail.com>
33c8925
to
a376841
Compare
Co-authored-by: Kyle Gach <kyle.gach@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still a little suspicious about the documentation on how to configure the router to use globals
. Otherwise the changes look great!
What I did
Support next/navigation in Next.js v13.
How to test
If your answer is yes to any of these, please make sure to include it in your PR.