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
[5.x] Augmentation Adjustments #9574
Closed
JohnathonKoster
wants to merge
7
commits into
statamic:master
from
JohnathonKoster:deferred-method-augmentation
Closed
[5.x] Augmentation Adjustments #9574
JohnathonKoster
wants to merge
7
commits into
statamic:master
from
JohnathonKoster:deferred-method-augmentation
Conversation
This file contains 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
edalzell
reviewed
Feb 23, 2024
37 tasks
Positive improvements across a variety of different situations, particularly the `group_by` modifier
11 tasks
jasonvarga
previously requested changes
Mar 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please target master
.
Closing in favor of #9636 |
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 PR adjusts some internal Augmentation logic. Its primary goal is delay resolving these original values as long as possible:
https://github.com/statamic/cms/blob/4.x/src/Data/AbstractAugmented.php#L51-L61
It also adds a new
toDeferredAugmentedArray
which will pass around anInvokableValue
object that will perform similar logic to what exists now, but only when the value is needed somewhere (whether that is via.raw
,value
, or any other method that uses those). This to augmentation method is intended to be internal, and have refactored some internal areas to use it instead (core modifiers, the Cascade's globals, etc.). Callingall()
on an existing augmented collection will resolve these invokable values like normal (such as for JSON responses, or third-party code relying on that behavior).Another big change was to reduce the number of calls to
blueprintFields
made. This was done by making a single call when usingselect
: https://github.com/statamic/cms/pull/9574/files#diff-3587a85d40c591a1d37c917c8eb3420091c9cd5203f6836bf71467323dde9deeR38-R43. Additional logic was also added to revert to calling it again when retrieving values usingget
directly, or via. other methods.A few notes:
dump
tag and modifier, but that can be sorted out separately if this all checks out (thinking to just force resolve those values when using the tag or modifier)wrapValue
method, but need to think on that some more and can be a separate item, if needed. This has been completed in [5.x] Augmentation performance improvements #9636