Skip to content

[5.3] Add the ability to disable caching of compiled Blade templates #13938

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

Closed
wants to merge 2 commits into from
Closed

Conversation

michaeldyrynda
Copy link
Contributor

Under some circumstances, such as when in development, you may opt to disable caching of compiled Blade templates. This change allows caching to be overwritten via a view configuration option.

Under some circumstances, such as when in development, you may opt to
disable caching of compiled Blade templates. This change allows caching
to be overwritten via a view configuration option.
@GrahamCampbell GrahamCampbell changed the title Add the ability to disable caching of compiled Blade templates [5.3] Add the ability to disable caching of compiled Blade templates Jun 10, 2016
@taylorotwell
Copy link
Member

I think somewhat proposed this before and I thought it always made more sense to support just setting the "compiled" option in views.php to null rather than inventing a new configuration option.

@michaeldyrynda
Copy link
Contributor Author

And just make an is_null check against sourcePath inside the isExpired method? Would need a change in the compile method as well I suspect.

You'd still need to compile the template, you just want the cache to always present as expired, right?

@taylorotwell
Copy link
Member

taylorotwell commented Jun 10, 2016 via email

@@ -73,8 +73,9 @@ public function registerBladeEngine($resolver)
// instance to pass into the engine so it can compile the views properly.
$app->singleton('blade.compiler', function ($app) {
$cache = $app['config']['view.compiled'];
$shouldCache = $app['config']['view.should_cache'] ?: true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't work. If I set should_cache to false it's just going to be true again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeh, this doesn't work

@vlakoff
Copy link
Contributor

vlakoff commented Jun 10, 2016

Already refs #12586 and #12587, and maybe there was another topic about this.

@michaeldyrynda
Copy link
Contributor Author

You're right @acasar. I did a little more digging based on Taylor's suggestion; setting the compiled option to null has some deeper reaching side effects, given that the view's render method still tries to load the cached contents from disk.

I'll do some more work on this this weekend.

@michaeldyrynda
Copy link
Contributor Author

michaeldyrynda commented Jun 12, 2016

I've done a little more digging; it seems that with a fresh 5.2 install if you set view.compiled to null as implemented in #12586, you'll wind up with an exception (which the unit tests don't pick up).

Exception when view.compiled is null

Subsequent commits here should address both that bug, as well as changes discussed in this PR. It doesn't appear as simple as testing for null, though, as you still need to compile the blade template at some point, which is the point of the cached file.

@vlakoff
Copy link
Contributor

vlakoff commented Jun 12, 2016

Maybe cachePath could be looked for "falsy" rather than just "null"?

@michaeldyrynda
Copy link
Contributor Author

Based on the above, I think that having a new config option may still be worthwhile, given the compiler engine requires including the compiled blade via path (and not passing a compiled string around). By the time execution gets to the engine, it has no knowledge of the compiled path being null or otherwise.

@taylorotwell
Copy link
Member

Not sure I'm going to end up adding this. There are already so many ways you could do it, such as a global middleware that checks env and runs view:clear or something.

@michaeldyrynda
Copy link
Contributor Author

Thanks @taylorotwell - I agree, there's a few other approaches to this. Although on further inspection, there shouldn't even theoretically be a (server-side) caching issues anyway, given the way expirations are handled internally.

@tillkruss
Copy link
Contributor

@taylorotwell Just setting view.compiled to null causes in a fatal error. Is there another way? Disabling the Blade cache on read-only system like Heroku is essential.

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.

6 participants