Pages are uncacheable because of the cache context user

This patch adds a setting that allows to optin for sending the user uid.

I did need to set _datalayer_defaults() inside datalayer_get_data_from_page() for datalayer to be propagated.

Setting this to needs review as there is a patch.

The last submitted patch, 2: remove-cache-context-user-3345817-2.patch, failed testing. View results →

This is a good idea. Making the user UID optional. And I support doing so.

For the UI though there's already an option to configure what data about a user is output under the "Current User Meta Data" heading in the "User Details" section of the settings form. And I think it would be potentially more intuitive if we add the uid field to that list. And then maybe also update the help text for that list with a note about how adding user details like this to the page can make the page uncachable so use with caution.

IMO this should default to off because its quite surprising to add/upgrade this module and have your cache break.

I think it should default to off for new installs for sure. The tricky part is upgrades, because the default behavior right now is to always add the user.uid. This patch makes that optional, but if we leave it off for upgrades we're potentially breaking someone's integrations if they rely on user.uid and it's suddenly not there.

IMO fixing their dynamic page cache is more important than preserving their existing behavior. You could make a new minor or major of the module if desired.

Actually we dont need a new config pref here. We should switch to using auto-placeholdering (aka lazy-builder). See https://medium.com/gridonic-web/understanding-drupals-auto-placeholderin... and https://drupalsun.com/philipnorton42/2022/05/01/drupal-9-using-lazy-buil....

Here is the #lazy_builder solution. As suggested, no new config is needed. I tried to keep using hook_page_attachments but thats incompatible with lazy_builder. The error is assert(): When a #lazy_builder callback is specified, no properties can exist; all properties must be generated by the #lazy_builder callback.. So I switched to using hook_page_bottom().

Moving to Major since this issue currently breaks dynamic page cache and page cache module.

The last submitted patch, 12: 3345817-12.diff, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Update test

That's a good fix! Thanks @moshe weitzman. I've tested this out locally and it seems to be working fine with my configuration. And the code looks fine. Committed to the 2.0.x branch and I'll roll a new (2.0.1) release shortly.

Automatically closed - issue fixed for 2 weeks with no activity.