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

fix: only forward required cookie header #1644

Merged
merged 4 commits into from
Nov 2, 2023
Merged

Conversation

juliusmarminge
Copy link
Member

@juliusmarminge juliusmarminge commented Nov 1, 2023

We only really need to forward the header so that tRPC and other stuff can authenticate the user. There are some issues with certain headers that we run into when sending every single header along (I know transfer-encoding and content-length are two that doesn't like playing nice)

Closes #1640
Closes #1641

Copy link

changeset-bot bot commented Nov 1, 2023

🦋 Changeset detected

Latest commit: e5136cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
create-t3-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Nov 1, 2023

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

Name Status Preview Comments Updated (UTC)
create-t3-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2023 9:18am
t3-upgrade ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2023 9:18am

Copy link
Contributor

github-actions bot commented Nov 1, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 96
🟢 Accessibility 91
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://create-t3-app-git-only-forward-cookie-t3-oss.vercel.app/

Copy link
Member

@c-ehrlich c-ehrlich left a comment

Choose a reason for hiding this comment

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

Looks good! I prefer this version of the fix because it means other headers won't cause similar issues.

Could we get @Allypost credited as a contributor when merging for profiling and submitting the initial solution?

edit: added as a co-author to the commit that added the fix

@github-actions github-actions bot added 📚 documentation Improvements or additions to documentation 📌 area: ci Relates to CI / GitHub Actions labels Nov 2, 2023
@c-ehrlich c-ehrlich added this pull request to the merge queue Nov 2, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 2, 2023
@c-ehrlich c-ehrlich added this pull request to the merge queue Nov 2, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 2, 2023
@juliusmarminge juliusmarminge added this pull request to the merge queue Nov 2, 2023
Merged via the queue into main with commit e6649f1 Nov 2, 2023
75 checks passed
@juliusmarminge juliusmarminge deleted the only-forward-cookie branch November 2, 2023 20:49
devvianto605 pushed a commit to devvianto605/create-devviantex-nextjs-app-deprecated that referenced this pull request Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📌 area: ci Relates to CI / GitHub Actions 📌 area: cli Relates to the CLI 📌 area: t3-app Relates to the generated T3 App 📚 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: tRPC breaks when using revalidatePath/revalidateTag in server action
2 participants