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

Cache Global-Styles #25680

Merged
merged 2 commits into from
Nov 13, 2020
Merged

Cache Global-Styles #25680

merged 2 commits into from
Nov 13, 2020

Conversation

aristath
Copy link
Member

Description

Adds a 1-minute cache to global styles - see #25678

On high-traffic sites this will avoid costly calculations and improve performance. Calculating styles once/minute is OK, calculating them on each page-request not so much. 😆

Styles will only be cached if we're not in debug more and not in the editor.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Sep 28, 2020

Size Change: 0 B

Total Size: 1.19 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.54 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.55 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 129 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.65 kB 0 B
build/block-library/editor.css 8.65 kB 0 B
build/block-library/index.js 145 kB 0 B
build/block-library/style-rtl.css 7.66 kB 0 B
build/block-library/style.css 7.65 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.6 kB 0 B
build/components/index.js 169 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.43 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 685 B 0 B
build/data/index.js 8.6 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.6 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.29 kB 0 B
build/edit-post/style.css 6.28 kB 0 B
build/edit-site/index.js 21 kB 0 B
build/edit-site/style-rtl.css 3.73 kB 0 B
build/edit-site/style.css 3.73 kB 0 B
build/edit-widgets/index.js 21.2 kB 0 B
build/edit-widgets/style-rtl.css 3.02 kB 0 B
build/edit-widgets/style.css 3.02 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@aristath aristath changed the title Cache for a minute using a transient Cache Global-Styles Sep 28, 2020
@mtias mtias added [Type] Performance Related to performance efforts Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Oct 1, 2020
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

We need to see the relation between caching and our extensibility solution. We may also need to have a solution to allow the browser caching of things in theme.json. Currently, that is not possible because we output everything as inline styles send on every page request. But what we have in this PR is better that what we have now so I guess we can merge it :)

if ( $can_use_cached ) {

// Check if we have the styles already cached.
$cached = get_transient( 'global_styles' );
Copy link
Member

Choose a reason for hiding this comment

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

When we have extensibility we need to document very well what plugins can do. If a plugin changes global styles depending on the page, for example, the caching solution makes things not work well because we have a global cache, so the value will be equal on all pages.

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

We need to see the relation between caching and our extensibility solution. We may also need to have a solution to allow the browser caching of things in theme.json. Currently, that is not possible because we output everything as inline styles send on every page request. But what we have in this PR is better that what we have now so I guess we can merge it :)

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

We need to see the relation between caching and our extensibility solution. We may also need to have a solution to allow the browser caching of things in theme.json. Currently, that is not possible because we output everything as inline styles send on every page request. But what we have in this PR is better that what we have now so I guess we can merge it :)

@aristath aristath merged commit 8e4b94d into master Nov 13, 2020
@aristath aristath deleted the try/global-styles-cache-25678 branch November 13, 2020 07:29
@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 13, 2020
@aristath aristath mentioned this pull request Nov 19, 2020
@mrleemon
Copy link
Contributor

Is this working correctly? Checking the global_styles transient with the Transients Manager plugin in WP 6.0 RC4 I see this transient as "Persistent" without a expiration date. So, any changes made to the theme.json file in a theme I'm developing are ignored unless I delete the transient.

@mrleemon
Copy link
Contributor

mrleemon commented May 22, 2022

I deleted the persistent transient, and now the new generated transient is set with an expiration date, so I'm no longer having issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants