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] Fix issue when using Livewire with full measure static caching #10306

Merged
merged 6 commits into from
Sep 25, 2024

Conversation

aerni
Copy link
Contributor

@aerni aerni commented Jun 14, 2024

I've worked on a couple of PR's for Jonas' Livewire addon to enhance support for Statamic's static caching. See jonassiewertsen/statamic-livewire#61 and jonassiewertsen/statamic-livewire#64.

The newly added AssetsReplacer adds support for Livewire's @assets Blade directive, that allows you to push assets into the <head>.

I've previously PR'd support for Livewire and Statamic's full measure static caching in #8762. This script works as expected in general scenarios. However, I've encountered an edge case, where the CSRF token isn't replaced in time before a request to Livewire is made.

This can happen, when a script, added to the <head> with the @assets directive, is making requests to Livewire, e.g. with this.$wire. The issue is that at the time of this request, the CSRF token hasn't been replaced yet, as the nocache script is only executed at the end of the </body>. So this.$wire is making a Livewire request using the STATAMIC_CSRF_TOKEN placeholder as the CSRF token.

This PR resolves this issue by moving the nocache JS to the head before any <link> or <script> so that the CSRF token placeholder is replaced before any other script is executed.

@aerni aerni changed the title Fix issue when using Livewire with full measure static caching [5.x] Fix issue when using Livewire with full measure static caching Jun 14, 2024
@duncanmcclean
Copy link
Member

Marking this PR as a draft for now. Once the tests are passing, feel free to mark this as "ready for review" and we can take a look. 👍

@duncanmcclean duncanmcclean marked this pull request as draft June 17, 2024 12:26
@aerni aerni marked this pull request as ready for review June 17, 2024 13:42
@aerni
Copy link
Contributor Author

aerni commented Jun 17, 2024

I've fixed the tests.

An alternative approach to the code changes could be to simply add the script at the beginning of the <head> instead. I leave this decision up to you.

@morhi
Copy link
Contributor

morhi commented Jun 18, 2024

Copied over from Discord:

I am working on integrating Livewire into a full static measure site. When using nocache tags the /!/nocache route will return an CSRF token, which is then replacing any STATAMIC_CSRF_TOKEN placeholder. Only after this point the Livewire components can be used, because they need the CSRF token in order to use the Livewire update route. In case the /!/nocache route takes a lot of time to respond, Livewire cannot be initialized until this point, also no custom alpine code, which don't need this token. What do you think about creating a separate route for getting the CSRF token to ensure a quick initialization, instead of getting the token and nocache contents via one single route, which might need some time to be fetched? The /!/nocache route could therefore omit the CSRF token.

Currently I am using this code to initialize Livewire:

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

@jasonvarga
Copy link
Member

That seems like a better approach to me. Even if we move the nocache js to the top of the page, the ajax request might still take longer than expected for a number of reasons. If you start Livewire only after the statamic:nocache.replaced is dispatched, it would work consistently.

@aerni
Copy link
Contributor Author

aerni commented Jun 18, 2024

I'm not sure if it's a good idea to defer Livewire.start() until statamic:nocache.replaced has been dispatched. Livewire can be started from the get-go, as it doesn't depend on the CSRF token. The token is only needed when an update request is sent. Deferring the start of Livewire also defers the start of Alpine, and would generally result in a slower initialization.

The only issue with the CSRF token is that a script might request an update to Livewire before the token has been fetched and replaced.

What do you think about creating a separate route for getting the CSRF token to ensure a quick initialization, instead of getting the token and nocache contents via one single route, which might need some time to be fetched? The /!/nocache route could therefore omit the CSRF token.

@jasonvarga Would this be a better solution? So we could have the CSRF token script in the head and the nocache script at the end of the body?

Also, worth considering; if Livewire allowed hooking into its update cycle, we could maybe add some code to tell Livewire to only execute the update requests once statamic:nocache.replaced has been dispatched. But not sure if this is possible.

@jasonvarga
Copy link
Member

Ok yeah that's all valid too.

@morhi
Copy link
Contributor

morhi commented Jun 18, 2024

The one issue, that we try to solve, are 419 errors if Livewire tries to update without a valid token, right?

@aerni
Copy link
Contributor Author

aerni commented Jun 18, 2024

Yes, that's the issue this PR tries to solve. 😄

@aerni aerni marked this pull request as draft June 19, 2024 14:24
@aerni
Copy link
Contributor Author

aerni commented Jun 19, 2024

I did some more testing, and it turns out that this PR works as expected when letting Livewire do its magic script injection, as per jonassiewertsen/statamic-livewire#64.

This PR also reduces the initial load times as mentioned by @morhi. So Livewire can start without having to wait for the CSRF token to be replaced.

However, if you are manually bundling Livewire, you are required to add this code shared above to your bundle:

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

If you don't, you will still run into a 419 error, as mentioned by @jasonvarga: "Even if we move the nocache js to the top of the page, the ajax request might still take longer than expected for a number of reasons."

I wish we could abstract this away somehow.

@aerni aerni marked this pull request as ready for review June 25, 2024 19:02
@jasonvarga
Copy link
Member

Ping @jasonvarga

@jasonvarga jasonvarga merged commit 563d731 into statamic:5.x Sep 25, 2024
16 checks passed
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.

5 participants