Add BF Cache management feature

[dadolun95]

No description provided.

[rhoerr]

Okay, so... this is really difficult for me. There's nothing strictly wrong with this pull request. The code is well done and you considered the situation and implications well with the config, VCL, and data section handling. I would ask for strict_types declarations and declaring all the dependencies in composer + module.xml.

But I have two PRs for bfcache here, and I think #5 is a better starting point given its config and JS/UX aspects. I'm going to move forward on processing #5 unless anything goes amiss. Would appreciate your input as well, if you're up for it.

Can you add the VCL modification aspect to #5?
Better idea: Let's wait until #5 is merged, and then we can slim this down to the VCL and section config parts and merge it as well.

[dadolun95]

Hi @rhoerr
As you requested is better to wait until #5 PR will be merged.
I'm going to add the VCL modifications when time comes.
Then what about reloading other customer data sections than cart? If I don't remember wrong #5 is able to close the menu inside the header and reloads cart section only, are we really sure that we don't need to reload messages and other sections also?
I think that customer data stored on browser can be used to check if certain section reload is needed instead of making one xhr call every time bf cache reload page event is triggered ... anyway ... Let's see all these points when #5 PR will be merged 👋

[rhoerr]

Thanks @dadolun95. I'm open to improvements on any of that, wherever it makes sense.

A comment from @JaJuMa on VCL #5:

Are you open to Davide incorporating the VCL aspect from #2?

We are open to adding this feature, but we believe it’s important to consider the implications carefully. Automatically generating a new VCL file for Varnish configuration could potentially cause confusion or conflicts with any existing VCL customizations that users may already have in place.

Perhaps we could consider adding a configuration note or mechanism to specify exactly which lines should be added and where, rather than generating a full new file. This way, we can help users integrate the changes safely into their existing setup.

Let’s discuss the best approach to ensure flexibility and avoid any unintended side effects for users with custom VCL configurations.

For further discussion.

[rhoerr]

I lean toward including out of box VCL updates as this proposes.

Overall it seems like something that should probably change in the core VCL template directly (with possible connection to https://github.com/mage-os/mageos-magento2/pulls?q=is%3Apr+is%3Aopen+vcl and/or https://github.com/elgentos/magento2-varnish-extended per Discord -- thanks @sprankhub and @dadolun95 ) -- but I want to see this available to any Magento instance that installs this module, too, if at all possible.

VCL customizations shouldn't be a direct concern, I think?, as this should only impact new VCL configuration files generated by Magento. But I don't generally use Varnish and don't have experience with it on various hosting environments or configurations. Please educate me here if this does introduce risks.

[convenient]

Just throwing in my two cents.

I do the varnish changes very similar to how they're done in the elgentos module here

https://github.com/elgentos/magento2-varnish-extended/blob/d77d3b415eec282b0f6127aff30886d0de1c9718/etc/varnish6.vcl#L283-L289

In fastly I have a snippet like

if (resp.http.Cache-Control !~ "private" && req.url !~ "^/(media|static)/" && fastly_info.state ~ "HIT") {
    set resp.http.Cache-Control = "must-revalidate, max-age=60";
}

In both these cases we have rules that read like "if the request is not private, not an asset, and is varnish cacheable" then "it's eligible for bfcache". The rationale being, if it was going to be a page load served by varnish anyway then it was already cacheable and shareable, so knock yourself out and use bfcache.

To me this is better than building some exclusion list, as you only have one ruleset for what is cacheable and it's the same for a "navigation" as well as a "back/forward". There's already rules in place for the checkout etc to make them not cacheable. This makes it a bit safer, as the same rules that are in place to determine whether a page is cacheable applies for both navigations as well as back/forward.

[JaJuMa]

Just a few clarifications on the current bfcache implementation:

The current implementation is not about maintaining an exclusion list.
Instead, it provides two key features:

• Robust eligibility logic
  The PHP plugin modifies Magento’s response headers based on its core FPC cacheable/not-cacheable signals,
  plus it provides flexibility to optionally exclude specific URLs for custom needs.
  (As explained in config note: "This configuration is optional for most sites. The extension automatically excludes non-cacheable URLs from back/forward cache. Use the blacklist below if you have custom cached URLs that load private data via JavaScript.")

  • This integrates seamlessly with the built-in cache.
  • For Varnish, the plugin still runs, but a corresponding VCL modification is required so that Varnish respects these headers.

• Crucial front-end safeguards
  JavaScript handlers mitigate potential security and UX issues by:

  • refreshing dynamic content (e.g. stale mini-cart),
  • restoring toggle state of open menus and the cart drawer,
  • forcing a full page reload if the user’s login state changes between visits.

These safeguards are essential regardless of the backend cache used.

---

Regarding the VCL snippet

The suggested version may not be the best fit, because:

• It depends on Fastly-specific conditions.
• It is quite strict (a cache HIT alone isn’t necessarily a valid reason to evict bfcache).
• It introduces an unrelated max-age header modification that could have unintended side effects on the browser's standard HTTP cache.

A safer approach that more closely mirrors the backend logic would be:

sub vcl_deliver {

// Update following line: //
set resp.http.Cache-Control = "no-store, no-cache, must-revalidate, max-age=0";

// To: //
    if (resp.http.Cache-Control ~ "public") {
        set resp.http.Cache-Control = "no-cache, must-revalidate, max-age=0";
    } else {
        set resp.http.Cache-Control = "no-store, no-cache, must-revalidate, max-age=0";
    }
}

---

Regarding the original question discussed here about including bfcache logic in .vcl generation

The main considerations from my point of view are operational:

• Developer awareness
  Tying this to VCL generation creates a manual dependency. Developers must re-generate the VCL when toggling the feature in admin:

  • Enabling without regeneration → feature won’t work.
  • Disabling without regeneration → site could be left with UX or security issues that the JS safeguards are designed to prevent.
    The fact that simplified, VCL-only suggestions often come up highlights the need for awareness of the full implementation, including these safeguards.

• Scope
  Any change should remain focused and targeted solely on bfcache requirements, without modifying or adding unrelated cache behavior.

[convenient]

Thanks @JaJuMa, for clarity the fastly and VCL examples were merely examples on how customisation can be applied by looking at the main caching rules. Not recommendations of how to implement here, there's no way a fastly snippet would be applicable here.

A lot of the feedback you posted above seems to pertain to the work implemented in #5, which seems sensible.

You say

The current implementation is not about maintaining an exclusion list.

but the work in this PR does seem to specifically be looking at a code defined excluded_paths which is what I was questioning?

<excluded_paths>checkout|cart|customer|wishlist|downloadable|sales|vault|review|newsletter</excluded_paths>

[JaJuMa]

@convenient Thanks for clarifying 🙏

I think the confusion here comes from PR #5 already being merged, which I was referring to as the “current implementation,” while PR #2 is still open due to the discussion about whether or not to include VCL generation logic for bfcache, and how.

That’s why my earlier comments may have sounded broader.
But the main points I raised still apply in both contexts (built-in cache, Varnish, and also Fastly when relevant).
I hope my points can be taken into account when moving forward with this module.

Hope that clears it up a bit!

[convenient]

No problems @JaJuMa , I did have to take a second look at this PR because I thought I missed a LOT of functionality somehow 😅

I haven't had time to read your initial work in detail, but maybe some flag or something can be set on the response for the VCL to look at, rather than this coded list? Just an idea. It would be good to avoid having multiple "exclude" lists.

[JaJuMa]

Thanks!

From my understanding, the open discussion here is mostly about the .vcl generation side of things.
Maybe it’s better for @rhoerr or @dadolun95 to answer if there are other points from PR #2 that should also be considered.

Regarding your question:

"but maybe some flag or something can be set on the response for the VCL to look at, rather than this coded list?"

That’s a great suggestion, and it’s actually how the module (as in current main branch with PR #5 merged) is already designed to work.
The module uses the Cache-Control: public header as the primary “flag” to signal to Varnish whether a page is eligible for bfcache.

[convenient]

That’s a great suggestion, and it’s actually how the module (as in current main branch with PR #5 merged) is already designed to work.
The module uses the Cache-Control: public header as the primary “flag” to signal to Varnish whether a page is eligible for bfcache.

In which case, the entire additional "exclude" functionality can be removed from this PR, as it will be driven by the definitions and the additional "optional exclude" that was already introduced in #5 ?

[dadolun95]

Hi guys,

In my point of view the vcl generated by admin must be congruent with any configuration made on admin that should impact varnish.

So speaking about bf-cache, if a site uses varnish proxy with auto-generated vcl by magento default will not work due to the vcl_deliver instruction provided by magento templates.
Of course developers must be aware of what varnish is and works and of course they need to upload the vcl manually so both documentation and vcl generation should be updated imho.

This PR is a draft of what we need to make in order to see bf-cache working with varnish updating vcl generation by admin also.
But a better example is elegentos module ( https://github.com/elgentos/magento2-varnish-extended ) I discover later than my PR thanks to @sprankhub that can be used as dependency.

So we can close this PR and ignore it but I think that vcl generation should be managed somehow and I think that the module previously mentioned is the more advanced opensource solution for that right now.

[JaJuMa]

I agree that the elgentos/magento2-varnish-extended module is a great initiative for improving the default VCL. We’ve even used parts of their work as inspiration before.

When it comes to pulling it in as a dependency for this module, though, there are a few considerations:

• Duplicate Configuration: Both modules expose an “Enable BFCache” option. If only one of them is toggled, you end up with a broken setup:
  Either BFCache enabled but without this module’s safeguards, or this module’s safeguards active but BFCache blocked by the default VCL. Two toggles for the same feature make misconfigurations very likely.

• Different Scope: elgentos’ module has a much broader scope, including enabling the standard HTTP cache as part of its BFCache configuration. For this project, I believe the goal should be a targeted, minimal solution focused specifically on BFCache. Enabling or disabling other caches should remain a deliberate choice, not something that happens implicitly when turning on BFCache.

• Developer Awareness: As you noted, developers do need to understand Varnish and manually upload the VCL. In that light, the most robust and least confusing approach may be to provide a clear, standalone VCL snippet in our documentation, rather than managing VCL generation through a dependency.

So while elgentos’ work is very valuable (and definitely worth recommending), my concern is that adding it as a dependency here might introduce more complexity - and risk - than it solves.

Given these points, doesn't it seem that referring to a tested VCL snippet in the docs (like the one mentioned above that cleanly mirrors the module's backend logic) would be a simpler and safer way to help Varnish users, while keeping this module itself focused solely on BFCache?

[rhoerr]

Thank you all for your input.

Okay, let's do this:

1. We'll consider direct changes to specific cache engines (Varnish/VCL, Fastly, ...) out of scope for this module.
   • Since changing Varnish config requires manual action anyway, which some hosts don't even allow, skipping it isn't going to have much practical impact, and will be less likely to confuse or conflict.
2. Add a comment/warning to the bfcache setting section to explain that support depends on the cache backend, and Varnish or other options may require additional configuration. Include a link to this module's readme.
3. Add configuration details/snippets for Varnish and Fastly to the readme for this module, based on discussions here. For Varnish, include a suggestion that people use https://github.com/elgentos/magento2-varnish-extended. (We may package that in the future, but not yet. No harm in suggesting it though.)

@JaJuMa in the interest of time, if you agree, could you make these changes in a new PR? I'd like this wrapped up in the next week, and Davide has some other modules/issues to handle.

@dadolun95 Is that good with you? Is there anything else you feel we need to address still?

[JaJuMa]

@rhoerr
Sure, I’ve created the PR here: #16

[rhoerr]

I'm going to close this now, as primary bfcache functionality was merged in #5, and we're handling revisions and docs changes in #15 and #16. If the data-section aspect is still a consideration, please open a new issue or PR for that aspect.

Thank you!