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

[5.x] Move nocache js back to end of body but make configurable #10898

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

jasonvarga
Copy link
Member

The Nocache JS was originally inserted at the end of the body, so any replacements happen after all the HTML is ready.

In #10306 the JS was moved into the head so it's ready before any Livewire. This broke non-Livewire usage.

This PR reverts it to the body by default, but adds a config option so that you can move it into the head if you need it for your situation.

Fixes #10890

@jasonvarga jasonvarga merged commit b3ce9e6 into 5.x Oct 3, 2024
17 checks passed
@jasonvarga jasonvarga deleted the fix-nocache-injection-position branch October 3, 2024 14:32
@ryanmitchell
Copy link
Contributor

Brill, thanks for sorting so quickly.

Just so I understand though, if you want it in the head (for livewire) this means any other nocache tags on the page wont work, ie you can't mix them. Surely the best solution for livewire would be to have it defined as a function (window.StatamicNoCacheReplace) so you could call it when you want?

@robdekort
Copy link
Contributor

Why would you want the nocache stuff in the head when you use livewire?

@edalzell
Copy link
Contributor

edalzell commented Oct 3, 2024

Why would you want the nocache stuff in the head when you use livewire?

On pages you don't use livewire, like a form.

@jasonvarga
Copy link
Member Author

Why would you want the nocache stuff in the head when you use livewire?
The conversation in the PR covers it. #10306

Just so I understand though, if you want it in the head (for livewire) this means any other nocache tags on the page wont work, ie you can't mix them.

Correct. I guess you go all in on Livewire.

Surely the best solution for livewire would be to have it defined as a function (window.StatamicNoCacheReplace) so you could call it when you want?

Probably, yes. A JS solution was offered in that PR where you can defer it to after nocache. I suppose they could do that too.

@robdekort
Copy link
Contributor

Appreciate the directions guys, but you’ve all lost me. I guess I’ll just bug you all when stuff breaks. So, nothing new basically 😂.

@jasonvarga
Copy link
Member Author

What's the issue? Something broke and we reverted it. It should be like nothing changed on your end.

@robdekort
Copy link
Contributor

Oh don’t worry, no issue at all. I’m just trying to keep up with changes. 😘

And now I’ll sort myself out.

@aerni
Copy link
Contributor

aerni commented Oct 5, 2024

Thinking about this some more.

As @ryanmitchell pointed out, you won't be able to have both Livewire and {{ nocache }} tags work with the current solution, which is a bummer and leaves a bit to be desired.

Having the nocache replacer script in the <head> is only ever needed when using Livewire's @assets directive to push scripts into the <head>, while also accessing this.$wire in that script. More on this here.

A simple solution is to bundle Livewire yourself and only start it once the token has been replaced. However, the problem with this is that the start of Alpine is delayed as well, leading to unnecessary delay.

if (window.livewireScriptConfig?.csrf === 'STATAMIC_CSRF_TOKEN') {
    document.addEventListener('statamic:nocache.replaced', () => Livewire.start());
} else {
    Livewire.start();
}

The best solution for this issue might be to have a separate route and script for getting the CSRF token to ensure a quick initialization. This CSRF script could be in the <head>. This should solve the Livewire issue. The /!/nocache route and script could therefore omit the CSRF token and be placed at the end of the <body>. This idea was initially pointed out by @morhi.

What do you think @jasonvarga?

duncanmcclean added a commit to statamic/statamic that referenced this pull request Oct 7, 2024
duncanmcclean added a commit to statamic/statamic that referenced this pull request Oct 7, 2024
* Nocache database driver

Related: statamic/cms#10671

* Prevent query parameters bloating the static cache

Related: statamic/cms#10701

* Add entry password protection

Related: statamic/cms#10800

* Fix small typo

Related: statamic/cms#10824

* Move nocache js back to end of body but make configurable

Related: statamic/cms#10898
aerni added a commit to aerni/cms that referenced this pull request Oct 25, 2024
The position option was only introduced in statamic#10898 to allow either Livewire or nocache to work. We can revert all this code as we’ve now got a separate CSRF route that will solve the Livewire issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using {{ nocache }} around a form renders it once, but not on subsequent visits
5 participants