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

Use Lazy Collection for artisan commands #3670

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

patrickomeara
Copy link
Contributor

Following on from #3669...

The artisan commands are the only place that the getBy*() and getOrphans*() methods are used within the package.

As MediaRepository is not yet configurable the only impacted users for this change would be those that replace it in the container, or those that resolve it and expect Eloquent Collections from thegetBy*() and getOrphans*() methods.

Alternatively, we could introduce an interface to bind to and a repository_class configuration option, and ship a CursorMediaRepository that returns LazyCollections from the getBy*() and getOrphans*() methods. I initially started down this path however, I can't think of a reason that the artisan commands shouldn't always use LazyCollection.

If it's decided that the latter option is better, we should take the opportunity of a major release to create a MediaCollections\MediaRepositories namespace.

I'm open to feedback.

* this way the models aren't hydrated until they are needed using far less memory.
* this change only affects the artisan commands `getCollection()` still returns a collection.
@freekmurze
Copy link
Member

For now, I don't mind those queries are defined multiple times. I'll re-evaluate this when creating a new major version.

@freekmurze freekmurze merged commit 5c3d40c into spatie:main Jul 18, 2024
9 checks passed
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