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

Feature request: Process bassetBlock scripts in basset internalize command #62

Open
jnoordsij opened this issue Jul 5, 2023 · 5 comments

Comments

@jnoordsij
Copy link
Contributor

Currently @bassetBlock directive are ignored by the internalize command.

It would be nice to have these assets also processed when running the internalize command, to have less operations when the first request lands.

@tabacitu
Copy link
Member

tabacitu commented Jul 5, 2023

Wait. This might be a documentation problem, rather than a real one 😀

AFAIK they're only ignored when Basset is running on localhost, or when the env variable BASSET_DEV_MODE=true. During dev mode, Basset tries to do as little as possible. In production, the @bassetBlock() will get cached.

You can try that out by doing BASSET_DEV_MODE=false in your .ENV file. Then either running php artisan basset:cache or browsing the page with the @bassetBlock()

Let me know.

@jnoordsij
Copy link
Contributor Author

I've checked on my local setup and I can't seem to get the cache command to cache the basset blocks, regardless of environment variable values. I noticed this, because I was comparing the .basset file right after running the cache command vs. the one after browsing through my admin panel, in which a few additional entries appeared.

Also, I noticed this line of code:

preg_match_all('/@(basset|bassetArchive|bassetDirectory)\((.+)\)/', $content, $matches);

My first look at that makes me think that bassetBlock directives are not handled (which I do sort of understand, since they would need a completely different kind of parsing compared to the others).

@tabacitu tabacitu assigned promatik and unassigned tabacitu Jul 6, 2023
@tabacitu
Copy link
Member

tabacitu commented Jul 6, 2023

hmmm_no_idea

@promatik ?

@promatik
Copy link
Contributor

promatik commented Jul 6, 2023

Exactly 🤷‍♂️
@bassetBlock is much more complex in terms of parsing, that's why it was ignored. And also because @bassetBlock is faster to be internalized during the load of a page, it doesn't require any download from CDNs or stuff like that.

But, it is possible, and it's a nice to have, it was just never a priority.
@tabacitu I'll let you decide on the priority of this one 👌

@tabacitu
Copy link
Member

tabacitu commented Jul 7, 2023

Oh I get it now. I think it's a SHOULD, but not urgent. Let's do it later on in the year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants