Skip to content

Lravel 13 compatibility and optional caching#149

Closed
jszd2022 wants to merge 14 commits into
nnjeim:masterfrom
jszd2022:master
Closed

Lravel 13 compatibility and optional caching#149
jszd2022 wants to merge 14 commits into
nnjeim:masterfrom
jszd2022:master

Conversation

@jszd2022

@jszd2022 jszd2022 commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

Medium changes

  • Fixed Laravel 13 compatibiliy: Laravel 13 no longer supports by default caching collections so I rewrote the caching to save collections as arrays (->toArray()) and collect() the cached data back to collections
  • Added withCaching() and withoutCaching() methods to WorldHelper::class to make caching optional; Caching default isEnabled can be set in config/world.php, after each BaseAction.execute() call the isEnabled resets to it's default value set in the config
  • Added optional cache=[0,1] query parameter to api calls to enable/disable cache for that call
  • Decoupled locale used by WorldHelper from the app locale; if not specified with WorldHelper::setLocale() the locale defaults to the app's locale, else they are separately settable.

Minor changes

  • Added Hungarian localization
  • Changed cacheKey generation to use app()->getLocale() instead of config(app.locale)

@jszd2022 jszd2022 marked this pull request as draft April 5, 2026 07:30
@jszd2022 jszd2022 marked this pull request as ready for review April 5, 2026 07:40
@nnjeim

nnjeim commented Apr 7, 2026

Copy link
Copy Markdown
Owner

@jszd2022 thank you for all the effort you did here.
please explain to me the rational behind your decision to make the caching optional.

@jszd2022

jszd2022 commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

The main idea is limiting storage usage, in case of file or database caching. In my use-case I cache a few often used country's states and cities, but don't cache the others, because they are rarely needed, so response time is not a priority.

@nnjeim

nnjeim commented Apr 9, 2026

Copy link
Copy Markdown
Owner

@jszd2022 I apologise in advance if i am asking you for more.
I would like us to split this PR into 2 if possible:
PR #1- Hungarian localisation and Laravel 13 compatibility
PR #2- Optional caching.

PR#1 we can merge right away.
PR#2 i would like to test it a bit more as it is more sensitive and it affects the overall architecture.

Thank you in advance

@jszd2022

jszd2022 commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

I separated the changes and created the respective PRs.
PR #150 - Laravel 13 compatibility and Hungarian localization
PR #151 - Optional caching
I also took the liberty to close this PR.

@jszd2022 jszd2022 closed this Apr 9, 2026
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.

2 participants