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

Fix @loadOnce directive #129

Merged
merged 2 commits into from
May 1, 2024
Merged

Fix @loadOnce directive #129

merged 2 commits into from
May 1, 2024

Conversation

pxpm
Copy link
Contributor

@pxpm pxpm commented May 1, 2024

Some weeks ago I fixed a broken compatibility of basset with @loadOnce directives in #122

That fixed the reported issue but left other cases un-handled, like @loadOnce blocks.

A search across our repos, I see we left some @loadOnce behind in PRO package. https://github.com/search?q=org%3ALaravel-Backpack+%40loadOnce%28&type=code

In some way, it kind of makes sense having some @loadOnce, since we didn't tell users to remove the loadOnce and use @basset as we did for most of our code, people who had published files with @loadOnce would expect them to still work.

As you see we only use @loadOnce for blocks of javascript, ence this fix is targeted at that. To expand the scope of this to support either css or js like bassetBlock does, we would need to parse the buffer (ob_get_clean()) and maybe search for <script> or <style> tags.

I could have also changed the @loadOnce directives we have in our code to @bassetBlocks, but that would still be a BC for users that have published those buttons before.

I think this gives us the better compatibility until we remove loadOnce completely in next version as we should have done when launching basset. 🤷 it is what it is.

@pxpm pxpm assigned promatik and pxpm May 1, 2024
@pxpm pxpm merged commit dc420ca into main May 1, 2024
5 checks passed
@pxpm pxpm deleted the loadOnce-fix branch May 1, 2024 13:44
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.

3 participants