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

[1.x] Prevents running route:cache on build #225

Merged
merged 1 commit into from
May 31, 2023

Conversation

joedixon
Copy link
Contributor

Running route:cache during the build step of an application can cause an issue.

Routes will be serialized when running this command using the app key set in the build envrionment (local machine, CI, etc) as the signature.

If the same key is not set in production, an error will occur when attempting to deserialize after the app is deployed.

@taylorotwell taylorotwell merged commit 7ddb67d into master May 31, 2023
@taylorotwell taylorotwell deleted the feat/prevent-route-cache branch May 31, 2023 14:50
@imrodrigoalves
Copy link

Hey @joedixon, is there any performance hit from not allowing to cache the routes anymore?

@joedixon
Copy link
Contributor Author

Hey @imrodrigoalves our tests haven't shown any tangible performance hit, but let me know if you run into any issues.

@Spharian
Copy link

Spharian commented Aug 1, 2023

Hey @joedixon,

I'm building an application with only with the basic resource methods (index, update, store,...) in my Controllers, which is great for code simplicity/readability but I end up with a lot of controllers and routes (almost 200 routes at the moment). I've read some tests talking about 20ms performance hit per request at this number of routes and my app will require more routes in the future. I'm pretty scared about adding routes and impacting performances too much as we can't cache routes in Laravel Vapor.

Is there any solution to allow routes caching?

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.

4 participants