Skip to content

/ gutenberg

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

Jump to bottom

Register Assets: cache bust without filemtime #29775

Open
wants to merge 5 commits into
base: trunk
from
Open

Register Assets: cache bust without filemtime #29775

wants to merge 5 commits into from
+26 −17

Conversation

@hellofromtonya
Copy link

@hellofromtonya hellofromtonya commented Mar 11, 2021 •
edited

Description

#18227 reports times when outdated assets are being served when updating the plugin. filemtime is suggested as the root cause. Though I was unable to reproduce, it is conceivable that the filemtime could cause this problem. Why?

The asset version strategy in the registration uses filemtime to generate a unique version based on the asset file's modification time. The return of filemtime is cached in the realpath cache with an expiration defined by the realpath_cache_ttl option in php.ini.

This PR replaces filemtime with 2 different strategies:

  • For production, it uses GUTENBERG_VERSION as the asset version.
  • For development, it uses current time in seconds via time() as the asset version.

Note:
time() is not cached in the realpath cache.

Benefits:

  • Simplifies the code
  • Avoids an extra hit to the filesystem (teeny tiny performance boost though user will never see it as it's in microseconds)
  • Mitigates the realpath cache scenario

How has this been tested?

Validated new version of the stylesheet loads when updating the plugin.

Steps:

  1. Installed/activated version 9.9.3
  2. In stylesheet build/build-directory/style/css, changed :root{--wp-admin-theme-color:#00ba3e;
  3. Noted asset's version ...gutenberg/build/block-directory/style.css?ver=1615418702
  4. Updated to new version with this PR's changes

Results:
The updated stylesheet was served to the browser.

  • The color changed to #007cba and rendered
  • Version changed ..gutenberg/build/block-directory/style.css?ver=1615469843

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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.
@hellofromtonya
Replaces filemtime when registering package styles.
Verified
This commit was signed with the committer’s verified signature.
hellofromtonya Tonya Mork
GPG key ID: 3577E0AFD2430B05 Learn about signing commits
37c80a7 
For production, uses GUTENBERG_VERSION.

For dev, uses microtime().
@hellofromtonya
Replaces filemtime when registering package scripts.
Verified
This commit was signed with the committer’s verified signature.
hellofromtonya Tonya Mork
GPG key ID: 3577E0AFD2430B05 Learn about signing commits
a53fafe 
For production, uses GUTENBERG_VERSION.

For dev, uses microtime().
@hellofromtonya
Improves comments.
Verified
This commit was signed with the committer’s verified signature.
hellofromtonya Tonya Mork
GPG key ID: 3577E0AFD2430B05 Learn about signing commits
Loading status checks…
30eaa13
@hellofromtonya hellofromtonya requested review from spacedmonkey and TimothyBJacobs as code owners Mar 11, 2021
@github-actions github-actions bot added the First-time Contributor label Mar 11, 2021
@github-actions
Copy link

@github-actions github-actions bot commented Mar 11, 2021

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @hellofromtonya! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.
@gziolo gziolo added the Gutenberg Plugin label Mar 11, 2021
@hellofromtonya
Use time() instead of microtime().
Verified
This commit was signed with the committer’s verified signature.
hellofromtonya Tonya Mork
GPG key ID: 3577E0AFD2430B05 Learn about signing commits
Loading status checks…
e415075
@aristath aristath mentioned this pull request Mar 11, 2021
Open
@mkaz
mkaz approved these changes Mar 11, 2021
View changes
Copy link
Member

@mkaz mkaz left a comment

The change looks good, tested and works fine.

The one caveat might be for Gutenberg developers who do not set the WP_DEBUG constant, we probably will want to confirm that is mentioned in the docs somewhere. This can be a separate PR.
@hellofromtonya
Copy link
Author

@hellofromtonya hellofromtonya commented Mar 11, 2021

The one caveat might be for Gutenberg developers who do not set the WP_DEBUG constant, we probably will want to confirm that is mentioned in the docs somewhere. This can be a separate PR.

What constant are Gutenberg devs more inclined to set, if any? The PR can check for either WP_DEBUG or ____.
@mkaz
Copy link
Member

@mkaz mkaz commented Mar 11, 2021

What constant are Gutenberg devs more inclined to set, if any? The PR can check for either WP_DEBUG or ____.

I'm not sure, my guess is none on the PHP side. The expectation is more setting in at build using something like:

$ NODE_ENV=development npm run build

Here is the dev setup document:
https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/getting-started-with-code-contribution.md
@hellofromtonya
Copy link
Author

@hellofromtonya hellofromtonya commented Mar 11, 2021 •
edited

That makes.

It's a reasonable expectation when working in npm run dev that dev versions will be served up. The WP_DEBUG constant is set when using wp-env https://github.com/WordPress/gutenberg/tree/trunk/packages/env#default-wp-config-values. In a separate PR we could add default set up for those not using wp-env.
@aristath
Copy link
Member

@aristath aristath commented Mar 12, 2021

What constant are Gutenberg devs more inclined to set, if any? The PR can check for either WP_DEBUG or ____.

Usually I have SCRIPT_DEBUG enabled... People working on Gutenberg do mostly JS work, so frequently that is set to true in order to get better debug info in the browser console
@hellofromtonya
Copy link
Author

@hellofromtonya hellofromtonya commented Mar 12, 2021 •
edited

Usually I have SCRIPT_DEBUG enabled... People working on Gutenberg do mostly JS work, so frequently that is set to true in order to get better debug info in the browser console

Thanks @aristath.

In looking at constants predefined when running wp-env, both are set to true:

WP_DEBUG: true,
SCRIPT_DEBUG: true,

SCRIPT_DEBUG is already being used as development flag in the same file.

Switching to use SCRIPT_DEBUG for consistency.
@hellofromtonya
For consistency, use SCRIPT_DEBUG as dev/prod flag.
Verified
This commit was signed with the committer’s verified signature.
hellofromtonya Tonya Mork
GPG key ID: 3577E0AFD2430B05 Learn about signing commits
Loading status checks…
f31f27b
@aristath
aristath approved these changes Mar 12, 2021
View changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkaz mkaz

@aristath aristath

@spacedmonkey spacedmonkey

@TimothyBJacobs TimothyBJacobs

Assignees
No one assigned
Labels
First-time Contributor Gutenberg Plugin
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@hellofromtonya @mkaz @aristath @gziolo