search

vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
MIT License
5.39k stars 943 forks source link

fix(cookies): Correctly apply global and per-route middlewares #2911

Closed gdarchen closed 1 week ago

gdarchen commented 1 week ago

Description

In a previous PR, I tried to fix an issue causing some per-route middlewares being broken when using the split cookies for the Admin and Shop APIs.

But since it was released in the 2.2.6, the cookies split is no longer working at all.

After investigating, it turned out we were still badly applying the per-route middleware:

In this PR, I made sure to:

  1. Apply a global cookies middleware, either using the right cookie name:
    • If the cookies have to be split, use the shop cookie name
    • Else, use the shared cookie name if provided
    • If not provided, default to session
  2. Only apply specific cookieSession middlewares on the /shop-api and /admin-api routes

Breaking changes

No breaking change.

Screenshots

https://github.com/vendure-ecommerce/vendure/assets/17927632/19fcbb66-513e-4072-bfe7-0b7d85eaa869

Checklist

📌 Always:

👍 Most of the time:

vercel[bot] commented 1 week ago

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

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Jun 19, 2024 3:34pm
gdarchen commented 1 week ago

[!NOTE] @michaelbromley I made a video to prove I could make it work properly on the dev-server. But that would be awesome if we could deploy like a canary version from a pull request (based on a label?) to test this version on our actual projects before merging it. Do you think it's something doable?

michaelbromley commented 1 week ago

Hi @gdarchen thanks for the quick turn around on this. I just published @vendure/core@2.2.7-canary.0 under the canary tag. Please let me know whether it works as intended and then we can publish the patch 👍

gdarchen commented 1 week ago

Hi @michaelbromley 👋 I just tested a deployment on our staging environment using the canary version you released and it seems to work as expected!

choim4389 commented 6 days ago

When will it be distributed to a stable version? This issue is an important feature of our product.

michaelbromley commented 6 days ago

Likely this week.