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

Version 4 #25

Closed
wants to merge 42 commits into from
Closed

Version 4 #25

wants to merge 42 commits into from

Conversation

Jasonej
Copy link
Contributor

@Jasonej Jasonej commented Nov 5, 2021

Version 4 seeks to make use of new functionality provided by PHP 8.0 and more strongly focus on formatting and a simpler, more extensible enhancement mechanism.

Changes without clutter from the clean slate commit can be found here.

Goals

Formatting

  • Drop requirement for a format method naming convention.
  • Support format aliases.
  • Explicitly defined defaults.
  • Implicit default when only format is present.
  • Allow for selection of specific formats.
  • Explicit default format on child class overrides default from parent class.
  • Integrate formatting with Resource.
  • Integrate formatting with ResourceCollection.
  • Integrate formatting with AnonymousResourceCollection.

Enhancements

  • Stack based enhancement application (similar to how state is applied with Eloquent factories).
  • Powered by a modify method.
  • Trait for the except enhancement.
  • Trait for the only enhancement.
  • Integrate enhancements with ResourceCollection.
  • Integrate enhancements with AnonymousResourceCollection.

Miscellaneous

  • Support specifying the intended response status code.
  • Resourceable trait.
  • Documentation.

Notes

  • PHP 8.0 is required.
  • The append enhancement is being dropped.
  • The call enhancement is being replaced by the core enhance method functionality which will power the enhancement mechanism.
  • The replace enhancement is being dropped.

@Jasonej Jasonej self-assigned this Nov 5, 2021
@erik-perri
Copy link
Member

Do any of the enhancement changes open the option to hook into the resource generation globally?

As an example, I'd like to force every resource in a project to return an automatically generated meta.type property.

@Jasonej
Copy link
Contributor Author

Jasonej commented Nov 6, 2021

Do any of the enhancement changes open the option to hook into the resource generation globally?

As an example, I'd like to force every resource in a project to return an automatically generated meta.type property.

I hadn't considered doing anything to support global, but I have been considering allowing for formats/enhancements for meta data. The with/additional data system provided by Laravel's resources is a little less straightforward, and the relationship between collections and meta data is something that would require some thought if we want to support enhancing it.

Do you have an idea for how you'd want to apply these global enhancements?

@erik-perri
Copy link
Member

My initial thought was having it function like View composers, but the more I have thought about it the less sure I am about it being a useful feature.

I have been trying to think of other data that makes sense to attach globally and have not though of anything else that wouldn’t require a big if instanceof block, which is probably not something we want to encourage.

I will think about it some more to see if I can come up with anything else that could sanely make use of global hooks or a better approach to applying something like generic meta type data to each resource.

@Jasonej Jasonej dismissed stale reviews from erik-perri and joeladam518 via 4825deb December 9, 2021 15:01
erik-perri
erik-perri previously approved these changes Dec 10, 2021
joeladam518
joeladam518 previously approved these changes Dec 29, 2021
frankspress
frankspress previously approved these changes Jan 13, 2022
README.md Outdated
The easiest way to handle this is to set the `$collects` property on the enhanced collection or ensure that you're
following the standard convention for resource and collection naming so that the `collects()` method can detect it.
use Sourcetoad\EnhancedResources\Enhancements\Only;
use Sourcetoad\EnhancedResources\Enhancements\Traits\HasExceptEnhancement;
Copy link

Choose a reason for hiding this comment

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

Should this be HasOnlyEnhancement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

Copy link

Choose a reason for hiding this comment

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

Sure. Excited about all this new stuff! I like how the features are becoming more explicit and transparent

jyann
jyann previously approved these changes Feb 10, 2022
@erik-perri
Copy link
Member

What is your opinion on resolving the AnonymousResourceCollection instance when creating a collection rather than using new?

In one of my projects, I previously created a custom collection resource handler to return the pagination meta information in a format I prefer. I decided to switch to the ::collection pattern using the pagination overrides Laravel now provides (see this PR). Doing so requires me to create both a Resource override with a collection method, and an AnonymousResourceCollection with the paginationInformation method. If it were resolved instead I could just create the AnonymousResourceCollection and use the container to switch the implementation. Not an issue if you'd prefer to leave it, one additional base class is not a big concern.

@Jasonej
Copy link
Contributor Author

Jasonej commented Feb 16, 2022

What is your opinion on resolving the AnonymousResourceCollection instance when creating a collection rather than using new?

Resolving it makes a ton of sense. I'll look into adding that in a bit.

@Jasonej Jasonej dismissed stale reviews from NilsDelaguardia and jyann via e1891e9 February 19, 2022 05:31
@Jasonej
Copy link
Contributor Author

Jasonej commented Feb 19, 2022

Added Laravel 9 support, resolving AnonymousResourceCollection instead of direct instantiation, and tagged as v4.0.0-rc1.

@joeladam518
Copy link
Member

This might need to be pushed to v5 because of #26. Adding Laravel 9 support required me to drop PHP7.4 and Laravel 7 support.

@Jasonej Jasonej closed this Mar 8, 2022
@Jasonej Jasonej deleted the v4 branch March 8, 2022 14:14
@Jasonej Jasonej mentioned this pull request Mar 8, 2022
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants