Skip to content

perf: replace getServerSessionForAppDir usages with our original getServerSession function#18897

Merged
PeerRich merged 4 commits intomainfrom
perf/remove-get-server-session-for-app-dir
Jan 26, 2025
Merged

perf: replace getServerSessionForAppDir usages with our original getServerSession function#18897
PeerRich merged 4 commits intomainfrom
perf/remove-get-server-session-for-app-dir

Conversation

@hbjORbj
Copy link
Copy Markdown
Contributor

@hbjORbj hbjORbj commented Jan 26, 2025 •
edited
Loading

What does this PR do?

  • Our original getServerSession has caching mechanisms in place + it doesn't pull large dependencies as getServerSessionForAppDir did
  • So this PR removes all usages of getServerSessionForAppDir and replaces them with our original getServerSession
  • I create buildLegacyRequest util (this is needed to use our original getServerSession) and use it as getServerSession({ req: buildLegacyRequest(...)}
  • small refactor—I move AdminLayout files and SettingsLayout files to App Dir

Test 1 — Visit /event-types as a not-signed in user (use Incognito). You should be redirected to /auth/login page

asdf.mov

Test 2 — /settings pages

Screen.Recording.2025-01-26.at.4.42.10.PM.mov

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Visit /event-types as a not-signed in user (use Incognito). You should be redirected to /auth/login page.
  • Test /settings pages

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 26, 2025 •
edited
Loading

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jan 26, 2025 9:31pm
calcom-web-canary ⬜️ Ignored (Inspect) Jan 26, 2025 9:31pm

@graphite-app graphite-app Bot requested a review from a team January 26, 2025 21:31
@keithwillcode keithwillcode added consumer core area: core, team members only labels Jan 26, 2025
@dosubot dosubot Bot added the performance area: performance, page load, slow, slow endpoints, loading screen, unresponsive label Jan 26, 2025
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Jan 26, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (01/26/25)

1 reviewer was added to this PR based on Keith Williams's automation.

@hbjORbj hbjORbj changed the title perf: remove get server session for app dir perf: replace getServerSessionForAppDir usages with our original getServerSession function Jan 26, 2025
hbjORbj
hbjORbj commented Jan 26, 2025
View reviewed changes
Comment thread packages/features/auth/lib/get-server-session-for-app-dir.ts
import { getServerSession } from "next-auth";
import { cookies } from "next/headers";

import { getOptions } from "./next-auth-options";
Copy link
Copy Markdown
Contributor Author

@hbjORbj hbjORbj Jan 26, 2025

Choose a reason for hiding this comment

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

  • this was pulling lots of large dependencies, creating a slow down
  • it's better to use our original getServerSession function (which has good caching mechanism in place)

Copy link
Copy Markdown
Contributor Author

@hbjORbj hbjORbj Jan 26, 2025

Choose a reason for hiding this comment

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

discussion: https://app.campsite.com/cal/posts/58hm2drix99s

@PeerRich PeerRich enabled auto-merge (squash) January 26, 2025 21:44
@PeerRich PeerRich merged commit 2b79ef0 into main Jan 26, 2025
@PeerRich PeerRich deleted the perf/remove-get-server-session-for-app-dir branch January 26, 2025 21:57
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 26, 2025

E2E results are ready!

ThyMinimalDev pushed a commit that referenced this pull request Jan 27, 2025
@hbjORbj @ThyMinimalDev
perf: replace getServerSessionForAppDir usages with our original `g…
9d306c8 
…etServerSession` function (#18897)
ThyMinimalDev added a commit that referenced this pull request Jan 27, 2025
@Ryukemeister @ThyMinimalDev
@emrysal @hbjORbj
feat: routing forms for api v2 (#18885)
e052758 
* init router endpoint for v2

* fixup fixup

* resolve merge conflicts

* update libraries

* update router controller

* make router endpoint public

* chore: refactor router endpoint and add team/org id

* cleanup

* chore: Only import Sentry when needed (#18882)

* perf: prerender routes in main navigation (#18883)

* chore: migrate /apps/categories pages (#18871)

* migrate apps/categories pages

* remove pages dir

* chore: app router - /payment pages (#18152)

* remove Head from payment page comp

* remove route from pages router

* move out of future folder

* add to config.matcher

* remove unneeded code

* update middleware

* fix

* fix

* perf: replace `getServerSessionForAppDir` usages with our original `getServerSession` function (#18897)

* chore: migrate /apps/installation pages (#18870)

* migrate apps/installation pages

* fix test

* chore: migrate `/auth/logout` (#18901)

* migrate /auth/verify-email page

* update middleware

* migrate /auth/logout page

* expose router endpoint

* small refactor pageprops not necessary

* sentry is already tracking performance

* reuse logic

* fixup! reuse logic

* fixup! fixup! reuse logic

* fixup! fixup! fixup! reuse logic

---------

Co-authored-by: Morgan Vernay <morgan@cal.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Benny Joo <sldisek783@gmail.com>
Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>
MuhammadAimanSulaiman pushed a commit to hit-pay/cal.com that referenced this pull request Feb 25, 2025
@hbjORbj @MuhammadAimanSulaiman
perf: replace getServerSessionForAppDir usages with our original `g…
7a2c2a7 
…etServerSession` function (calcom#18897)
MuhammadAimanSulaiman pushed a commit to hit-pay/cal.com that referenced this pull request Feb 25, 2025
@Ryukemeister @ThyMinimalDev
@emrysal @hbjORbj @MuhammadAimanSulaiman
feat: routing forms for api v2 (calcom#18885)
820bfd7 
* init router endpoint for v2

* fixup fixup

* resolve merge conflicts

* update libraries

* update router controller

* make router endpoint public

* chore: refactor router endpoint and add team/org id

* cleanup

* chore: Only import Sentry when needed (calcom#18882)

* perf: prerender routes in main navigation (calcom#18883)

* chore: migrate /apps/categories pages (calcom#18871)

* migrate apps/categories pages

* remove pages dir

* chore: app router - /payment pages (calcom#18152)

* remove Head from payment page comp

* remove route from pages router

* move out of future folder

* add to config.matcher

* remove unneeded code

* update middleware

* fix

* fix

* perf: replace `getServerSessionForAppDir` usages with our original `getServerSession` function (calcom#18897)

* chore: migrate /apps/installation pages (calcom#18870)

* migrate apps/installation pages

* fix test

* chore: migrate `/auth/logout` (calcom#18901)

* migrate /auth/verify-email page

* update middleware

* migrate /auth/logout page

* expose router endpoint

* small refactor pageprops not necessary

* sentry is already tracking performance

* reuse logic

* fixup! reuse logic

* fixup! fixup! reuse logic

* fixup! fixup! fixup! reuse logic

---------

Co-authored-by: Morgan Vernay <morgan@cal.com>
Co-authored-by: Alex van Andel <me@alexvanandel.com>
Co-authored-by: Benny Joo <sldisek783@gmail.com>
Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeerRich PeerRich PeerRich approved these changes

Assignees

No one assigned

Labels

consumer core area: core, team members only performance area: performance, page load, slow, slow endpoints, loading screen, unresponsive ready-for-e2e

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hbjORbj @PeerRich @keithwillcode