-
Notifications
You must be signed in to change notification settings - Fork 149
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
[SDK-4499] Update to use the app router #131
Conversation
b5d0c32
to
b3fc7a2
Compare
@@ -2,7 +2,7 @@ | |||
* @jest-environment node |
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.
Should we update the dir structure of the tests?
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.
Done e2220b5
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.
With these changes, the sample app will no longer serve as an example of Next.js SDK + pages router, only with the app router, right?
Should we not showcase both, given that both are supported, and are likely being used in many real world apps due to incremental migration to the app directory?
Correct
I disagree, there are many use cases not covered in this sample - including incremental migration. Adding support for pages would add too much bloat and would not represent a simple sample app. (we don't for example cover middleware in the sample either). For the swiss-army knife, cover all example - we have the example app in the repo |
I don't mean for the sample app to cover all use cases (it's not meant for that) but to cover login, logout, and show user profile with both routers (same use cases covered in the QS, for which this is the companion sample app). |
BTW, I don't think I mentioned that the sample app needs to cover all use cases 🤔 I wonder what gave you that impression. |
See also auth0/docs#10293