Thread blade compiler instance through pipeline#95
Merged
Conversation
Contributor
Benchmark Results
Median of 10 attempts (* = outlier, excluded from result), 5000 iterations x 10 rounds, 49.93s total |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The scenario
Running
php artisan optimizewith Sentry installed produces broken compiled templates — components aren't compiled to functions.The problem
During
php artisan optimize, Laravel runs multiple commands sequentially:config:cachestep swaps the application instance in the container.view:cacheruns next, the blade compiler doing the compilation belongs to the original instance.app('blade.compiler')anywhere resolves from the new instance which is different from the one actually doing the compilation.This only happens if the blade engine was resolved before
view:cacheruns. Without the early resolution, Laravel would compile views using the blade compiler from the new app instance. Any package can trigger this early resolution — Sentry does this while decorating the blade engine.The blade compiler carries state — current path, raw blocks, etc. — that we rely on throughout compilation.
In #43,
app('blade.compiler')->getPath()returned null because it resolved the wrong instance, causing downstream bugs.The solution
The immediate fix was shipped in #88.
This PR prevents the entire class of issue by ensuring Blaze always uses the same blade compiler instance throughout compilation.
We refactored
BladeServicefrom a static utility class into a singleton that receives the blade compiler via constructor injection. That instance is threaded through the entire pipeline — no class resolvesapp('blade.compiler')from the container anymore. This required widescale refactors to decouple classes that previously depended onBladeServiceor resolved the compiler themselves.Added an integration test that verifies no class resolves
blade.compilerfrom the container during compilation.