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

[5.x] Cache results of getComputedCallbacks for a given scope #9640

Merged
merged 3 commits into from
Mar 13, 2024
Merged

[5.x] Cache results of getComputedCallbacks for a given scope #9640

merged 3 commits into from
Mar 13, 2024

Conversation

JohnathonKoster
Copy link
Contributor

This PR caches the results of getComputedCallbacks for a given scope

@JohnathonKoster JohnathonKoster mentioned this pull request Mar 2, 2024
37 tasks
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

Please target master and adjust the PR title to show [5.x].

@JohnathonKoster JohnathonKoster changed the base branch from 4.x to master March 8, 2024 01:06
@JohnathonKoster JohnathonKoster changed the title [4.x] Cache results of getComputedCallbacks for a given scope [5.x] Cache results of getComputedCallbacks for a given scope Mar 8, 2024
@jasonvarga jasonvarga dismissed their stale review March 11, 2024 16:56

Requested changes were made.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

I don't think this is suitable. Maybe if the blink key also contained the class name.

I know at the moment we only use this trait on collections, but lets say we also used it on taxonomies...

Collection::getComputedCallbacks('foo');
Taxonomy::getComputedCallbacks('foo');

These would both return the same items since they share a blink key.

@JohnathonKoster
Copy link
Contributor Author

I don't think this is suitable. Maybe if the blink key also contained the class name.

I know at the moment we only use this trait on collections, but lets say we also used it on taxonomies...

Collection::getComputedCallbacks('foo');
Taxonomy::getComputedCallbacks('foo');

These would both return the same items since they share a blink key.

Blink keys are now prefixed with the calling class name 👍

@jasonvarga jasonvarga merged commit 4064e25 into statamic:master Mar 13, 2024
16 checks passed
@jasonvarga jasonvarga deleted the cache-computed-callbacks-for-scope branch March 13, 2024 01:30
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