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

Bail out of static rendering for pages and routes in app dir #1541

Merged
merged 8 commits into from
Nov 13, 2023

Conversation

adamjmcgrath
Copy link
Contributor

@adamjmcgrath adamjmcgrath commented Nov 7, 2023

📋 Changes

Ensure getConfig doesn't run when the Next.js app is being statically rendered (because static rendering happens at build time and users don't necessarily want to supply the config at build time)

The 2 large commits are for making sure the config can be lazily evaluated at request time (config -> getConfig(req)):

  • 30c100e allow lazy config in auth0-session
  • d79ff8f allow lazy config in Next.js layer

The 3rd small commit is using this to bail out of static rendering before getting the config.

bec7d6c - Bail out of static rendering for pages and routes in app dir

You can bail out of static rendering by using the cookies() helper for Server Components and inspecting the request's url for API routes

📎 References

fixes #1356
vercel/next.js#49006

🎯 Testing

  • Make sure you have no environment variables
  • Run "npm run build:example"
  • You should see no errors

Copy link

vercel bot commented Nov 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nextjs-auth0 🔄 Building (Inspect) Visit Preview 💬 Add feedback Nov 7, 2023 1:30pm

@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4306696) 100.00% compared to head (ed41013) 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##              main     #1541    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           63        62     -1     
  Lines         6558      6293   -265     
  Branches       487       500    +13     
==========================================
- Hits          6558      6293   -265     
Files Coverage Δ
src/auth0-session/client/abstract-client.ts 100.00% <100.00%> (ø)
src/auth0-session/client/edge-client.ts 100.00% <100.00%> (ø)
src/auth0-session/client/node-client.ts 100.00% <100.00%> (ø)
src/auth0-session/handlers/callback.ts 100.00% <100.00%> (ø)
src/auth0-session/handlers/login.ts 100.00% <100.00%> (ø)
src/auth0-session/handlers/logout.ts 100.00% <100.00%> (ø)
src/auth0-session/session/abstract-session.ts 100.00% <100.00%> (ø)
src/auth0-session/session/stateful-session.ts 100.00% <100.00%> (ø)
src/auth0-session/session/stateless-session.ts 100.00% <100.00%> (ø)
src/auth0-session/transient-store.ts 100.00% <100.00%> (ø)
... and 12 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adamjmcgrath
Copy link
Contributor Author

adamjmcgrath commented Nov 7, 2023

Investigating an issue with the edge client fixed in 62a2626

Copy link

vercel bot commented Nov 10, 2023

You must have Developer access to commit code to Auth0 on Vercel. If you contact an administrator and receive Developer access, commit again to see your changes.

Learn more: https://vercel.com/docs/concepts/teams/roles-and-permissions#enterprise-team-account-roles

@dominicfallows
Copy link

This works great for my projects - tested on several. Thanks for sorting this, hopefully this can be made official soon?

@adamjmcgrath adamjmcgrath marked this pull request as ready for review November 13, 2023 09:10
@adamjmcgrath adamjmcgrath requested a review from a team as a code owner November 13, 2023 09:10
sessionCache: SessionCache
): GetAccessToken {
return async (reqOrOpts?, res?, accessTokenRequest?): Promise<GetAccessTokenResult> => {
const options = (res ? accessTokenRequest : reqOrOpts) as AccessTokenRequest | undefined;
const req = (res ? reqOrOpts : undefined) as IncomingMessage | NextApiRequest | undefined;
// TODO: clean up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be removed or is there work to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yep, will remove that comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment around the todo

@adamjmcgrath adamjmcgrath merged commit 456ec7c into main Nov 13, 2023
13 of 14 checks passed
@adamjmcgrath adamjmcgrath deleted the lazy-conf branch November 13, 2023 14:40
@adamjmcgrath adamjmcgrath mentioned this pull request Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

next build failing with missing environment variables
4 participants