-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Make it possible to set cookies client-side #25670
Comments
Cookie support would be great! Example use cases I have bumped into include:
Need to be clear on what restrictions exist for AMP Cache vs origin domain names in URLs and cookies. A cookie set on the origin will be not available if a page is served from the AMP Cache? Side note: Elsewhere I had wondered if it made sense to associate AMP state with cookies. AMP state is initialized to selected cookies on page load (making it accessible from to render after initial page load). Setting AMP state bound to cookies updated the cookie. |
Implementing |
I don't see a problem with this, but I'm not a good authority on Cookies. @jridgewell, thoughts on allowing |
Needs a privacy review. |
Makes sense. How do we kick that off? |
Search for SPUR in moma, we have to file ariane tickets, etc. |
I don't know enough about how AMP.getCookie() would be made available, but just a reminder I think a valuable use case is for the cookie to affect rendering of the page on initial page load. E.g. personalization - inject the user's name if returning, or display preferences based on segmentation knowledge from previous sessions etc. This is why I liked the idea of saying "persist this part of AMP state in a cookie". If the AMP state is updated, update the cookie. On page load, restore that AMP state from the cookie before #26473 kicks in feeding AMP state into amp-list rendering on page load. (I am sure there are other ways of doing it, but getting cookie values on page load into amp-list or similar I think is valuable. I want the original page render affected by the cookie value.) |
+1 to that idea too - the ability to serialize a state var into a cookie, then reverse the process on subsequent page load |
I hadn't been thinking about such large state variables! localStorage is an interesting idea... |
I put together a draft PR for what is probably the simplest approach to adding As for |
Thanks Matt. As a meta point, I wonder if promotion of cookies might be a future foot-gun WRT on-cache vs. off-cache, given the direction of 3P cookies e.g. https://webkit.org/blog/10218/full-third-party-cookie-blocking-and-more/. Though there's still value for AMP first/dirty AMP/signed exchanges. |
This is a good point. Cookies were tough before in the world of the AMP cache, and I don't think we're going to see signed exchanges everywhere anytime soon. On the other hand, we are trying hard to promote the idea of AMP-first! And I think the number more of people who will struggle with AMP because they can't use cookies is greater than the number who will have trouble with cookies vanishing on the cache. So I'm personally in favor of pursuing this. If others aren't, though - if we think cookies are going to become a dinosaur - I think we should simply be public about that in our documentation and outreach. |
Sadly my contact with developers isn't what it was before March 🙄 As I think about requests for this that I heard last year and the prior year, I don't feel like I heard people asking for just get or set. I remember requests related to analytics, requests related to persisting state without having to make server calls... and simple client-side personalization. I also remember that @alankent was interested in remembering logged-in state. If those are still the use cases, "set cookie" will make some of that functionality easier, so it's probably better than nothing :) Of course it might be easier to do this with |
You can do a lot with cookies/localstorage, but I'd like to use either to simply remember preferences a user selects such as a preferred dark/light mode css, so I'd love to see this feature implemented. |
Makes sense! @codysaylor , if it helps for the time being, |
Hello. Any news about cookies? |
I think we'd need a volunteer to follow up on @mdmower 's exploration. |
This is a bit speculative, but I imagine the reason cookies-for-AMP hasn't gotten traction is creators would be disappointed by limitations. Reading and writing cookies would likely need to be restricted to source origins (e.g. allowed on example.com, but not on google.com/amp/s/example.com). If all AMP pages served from AMP caches were delivered as signed exchanges (wouldn't that be cool?!), we'd be all set. As it is though, with so many AMP pages viewed from google.com/amp, this cookie feature would lead to differences in capabilities between pages served via an AMP viewer and those served from source origins. EDIT: I suppose I should have re-read this thread before posting. This concern has been mentioned a few times 😏 . So, nothing new here... but probably valid nonetheless. |
it's still an important point @mdmower ! 😎 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
We hear this request often from more sophisticated and experienced AMP site creators - to allow reading and setting of cookies from
<amp-bind>
,<amp-list>
, and/or<amp-analytics>
.A common workaround is to set cookies server-side, which sometimes makes it necessary to create an API to set cookies. We want AMP to make things easier for developers, and unfortunately this server-side workaround makes the experience harder for developers and users alike.
Of course, being able to set cookies from
<amp-script>
would be nice too, and might cover these use cases. But it would be great to add this to older interactive components as, of course, they're far more widely adopted at the moment.One possible format:
on="tap:AMP.setCookie({currency: 'USD'})"
/cc @mattwomple , @SimonHogstromRonbol, @nainar, @choumx
The text was updated successfully, but these errors were encountered: